1637199 - Build failure due to #include <version> picking up js/src/new-regexp/VERSION on macOS (unknown type name 'Imported', expected unqualified-id)

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

I just updated my local checkout on macOS from hg rev 19e273db80195cc5de59647fcaf16bafad9bbcce to hg rev 1706d4d54ec68fae1280305b70a02cb24c16ff68 and started getting this build error:

 9:38.12    Compiling webrender_api v0.61.0 (/Users/kats/zspace/gecko-mac/gfx/wr/webrender_api)
10:14.17    Compiling style v0.0.1 (/Users/kats/zspace/gecko-mac/servo/components/style)
11:06.85 js/src/wasm
12:39.40    Compiling webrender v0.61.0 (/Users/kats/zspace/gecko-mac/gfx/wr/webrender)
12:39.49 js/src/new-regexp
12:43.40 In file included from /Users/kats/zspace/gecko-mac/js/src/new-regexp/property-sequences.cc:7:
12:43.40 In file included from /Users/kats/zspace/gecko-mac/js/src/new-regexp/property-sequences.h:10:
12:43.40 In file included from /Users/kats/zspace/gecko-mac/js/src/new-regexp/regexp-shim.h:14:
12:43.40 In file included from /Users/kats/zspace/gecko-mac/obj-host-debugopt/dist/include/mozilla/Assertions.h:53:
12:43.40 In file included from /Users/kats/.mozbuild/clang/bin/../include/c++/v1/stdlib.h:100:
12:43.40 In file included from /Users/kats/.mozbuild/clang/bin/../include/c++/v1/math.h:311:
12:43.40 In file included from /Users/kats/.mozbuild/clang/bin/../include/c++/v1/type_traits:417:
12:43.40 In file included from /Users/kats/.mozbuild/clang/bin/../include/c++/v1/cstddef:37:
12:43.40 /Users/kats/zspace/gecko-mac/js/src/new-regexp/version:1:1: error: unknown type name 'Imported'
12:43.40 Imported using import-irregexp.py from:
12:43.40 ^
12:43.40 /Users/kats/zspace/gecko-mac/js/src/new-regexp/version:1:10: error: expected unqualified-id
12:43.40 Imported using import-irregexp.py from:
12:43.40          ^

I'm on a case-insensitive file system, and there's a VERSION file in js/src/new-regexp/ that's been there for a while. But now for some reason the #include <version> in the std cstddef header is picking it up, causing a build error. Fun!

Probably a regression from bug 1634135 although I haven't confirmed.

The mozconfig I'm using can be seen at https://github.com/staktrace/moz-scripts/blob/3de933b74c21e4cb0c1a7a879dbbcbe58ce38ea2/mozconfig.Darwin-host-debugopt

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Running ./mach build -j1 --verbose produces this as the failing command:

/usr/local/bin/ccache /Users/kats/.mozbuild/clang/bin/clang++ -isysroot /Users/kats/zspace/MacOSX-SDKs/MacOSX10.11.sdk -isysroot /Users/kats/zspace/MacOSX-SDKs/MacOSX10.11.sdk -std=gnu++17 -o property-sequences.o -c  -fvisibility=hidden -fvisibility-inlines-hidden -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -ftrivial-auto-var-init=pattern -DDEBUG=1 -DWASM_SUPPORTS_HUGE_MEMORY -DJS_CACHEIR_SPEW -DJS_STRUCTURED_SPEW -DJS_HAS_CTYPES -DFFI_BUILDING -DEXPORT_JS_API -DMOZ_HAS_MOZGLUE -I/Users/kats/zspace/gecko-mac/js/src/new-regexp -I/Users/kats/zspace/gecko-mac/obj-host-debugopt/js/src/new-regexp -I/Users/kats/zspace/gecko-mac/obj-host-debugopt/js/src -I/Users/kats/zspace/gecko-mac/js/src -I/Users/kats/zspace/gecko-mac/obj-host-debugopt/dist/include -I/Users/kats/zspace/gecko-mac/obj-host-debugopt/dist/include/nspr -fPIC -DMOZILLA_CLIENT -include /Users/kats/zspace/gecko-mac/obj-host-debugopt/js/src/js-confdefs.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-noexcept-type -Wno-unknown-warning-option -fno-sized-deallocation -fno-aligned-new -fno-common -stdlib=libc++ -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /Users/kats/zspace/gecko-mac/obj-host-debugopt/build/clang-plugin/libclang-plugin.dylib -Xclang -add-plugin -Xclang moz-check -O3 -fno-omit-frame-pointer -funwind-tables -Werror -fno-strict-aliasing -Werror=format -Wno-shadow -DV8_INTL_SUPPORT  -MD -MP -MF .deps/property-sequences.o.pp   /Users/kats/zspace/gecko-mac/js/src/new-regexp/property-sequences.cc

Comment 2

[Iain Ireland [:iain]]

The easy fix is for me to change the name of that file.

Comment 3

[Nick Alexander :nalexander [he/him]]

Folks, for unrelated reasons I finally ran this issue down. (We've seen it in many different guises.) Sadly, I don't really know what is to be done.

What I believe is happening is an interaction between our system header wrapping, case-insensitive file systems, and clang's permissive #include{_next} handling. First, our system header wrapping tries to wrap some headers with two cases (i.e., string.h and String.h) which is bound to not do the right thing on case-insensitive file systems. But even if we were to make some sense of that, we'd still have the reality that we could wrap a header (like "version.h") and have it redirect to a different header (like "Version.h") somewhere else. That's my guess as to what kats is seeing.

The way I traced this down was to run the preprocessor with line annotations (-P, I think). In my case I could see a benign "debug.h" #include_next-ing into some unrelated system header.

There are flags in clang to warn about these "non-portable" headers, but it doesn't appear possible to force clang to only accept headers that match case. (And indeed, the Windows SDK is littered with includes that assume case-insensitivity.)

So I have no good ideas about what to do. Do we still need the system header wrapping for visibility now that we use much more modern clangs?

Comment 4

[Iain Ireland [:iain]]

Attachment: Bug 1637199: Change name of irregexp version file r=mgaudet DONTBUILD

On OSX, mach is picking this up for #include <version>.

Comment 5

[Nathan Froyd [:froydnj]]

(In reply to Nick Alexander :nalexander [he/him] from comment #3)

So I have no good ideas about what to do. Do we still need the system header wrapping for visibility now that we use much more modern clangs?

Yes, we do.

We have a linter for mingw capitalization: https://firefox-source-docs.mozilla.org/code-quality/lint/linters/mingw-capitalization.html which tries to catch this sort of issue for Windows headers. I guess we should add the std:: headers to it as well?

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/263182d93843
Change name of irregexp version file r=mgaudet DONTBUILD

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/263182d93843