Closed
Bug 1472569
Opened 6 years ago
Closed 6 years ago
Recent change in js/ fails on gcc 8
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: Sylvestre, Assigned: Waldo)
References
Details
Attachments
(1 file)
/usr/bin/g++-8 -o RegExp.o -c -I/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/system_wrappers -include /root/firefox-gcc-last/config/gcc_hidden.h -DDEBUG=1 -DENABLE_BINARYDATA -DENABLE_SIMD -DENABLE_WASM_BULKMEM_OPS -DENABLE_WASM_SATURATING_TRUNC_OPS -DENABLE_WASM_THREAD_OPS -DENABLE_WASM_GC -DWASM_HUGE_MEMORY -DJS_CACHEIR_SPEW -DENABLE_SHARED_ARRAY_BUFFER -DEXPORT_JS_API -DJS_HAS_CTYPES '-DDLL_PREFIX="lib"' '-DDLL_SUFFIX=".so"' -DFFI_BUILDING -DMOZ_HAS_MOZGLUE -I/root/firefox-gcc-last/js/src -I/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/js/src -I/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/js/src/ctypes/libffi/include -I/root/firefox-gcc-last/js/src/ctypes/libffi/src/x86 -I/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include -I/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/nspr -fPIC -DMOZILLA_CLIENT -include /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/js/src/js-confdefs.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wc++1z-compat -Wduplicated-cond -Wimplicit-fallthrough -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wno-error=multistatement-macros -Wno-error=class-memaccess -Wformat -Wformat-overflow=2 -Wno-noexcept-type -fno-sized-deallocation -Wno-error=class-memaccess -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O3 -fno-omit-frame-pointer -Werror -Wno-shadow -Werror=format -fno-strict-aliasing -MD -MP -MF .deps/RegExp.o.pp /root/firefox-gcc-last/js/src/builtin/RegExp.cpp In file included from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Atomics.h:19, from /root/firefox-gcc-last/js/src/jsfriendapi.h:10, from /root/firefox-gcc-last/js/src/gc/Tracer.h:10, from /root/firefox-gcc-last/js/src/vm/TaggedProto.h:10, from /root/firefox-gcc-last/js/src/gc/Marking.h:16, from /root/firefox-gcc-last/js/src/vm/RegExpObject.h:16, from /root/firefox-gcc-last/js/src/builtin/RegExp.h:10, from /root/firefox-gcc-last/js/src/builtin/RegExp.cpp:7: /root/firefox-gcc-last/js/src/frontend/TokenStream.h: In member function 'bool js::frontend::TokenStreamChars<char16_t, AnyCharsAccess>::getFullAsciiCodePoint(int32_t, int32_t*)': /root/firefox-gcc-last/js/src/frontend/TokenStream.h:1467:40: error: 'using CharsBase = class js::frontend::TokenStreamCharsBase<char16_t>' {aka 'class js::frontend::TokenStreamCharsBase<char16_t>'} has no member named 'previousCodeUnit'; did you mean 'peekCodeUnit'? MOZ_ASSERT(lead == sourceUnits.previousCodeUnit(), ^~~~~~~~~~~~~~~~ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:417:69: note: in definition of macro 'MOZ_VALIDATE_ASSERT_CONDITION_TYPE' static_assert(mozilla::detail::AssertionConditionType<decltype(x)>::isValid, \ ^ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:450:39: note: in expansion of macro 'MOZ_ASSERT_HELPER2' #define MOZ_RELEASE_ASSERT_GLUE(a, b) a b ^ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:452:3: note: in expansion of macro 'MOZ_RELEASE_ASSERT_GLUE' MOZ_RELEASE_ASSERT_GLUE( \ ^~~~~~~~~~~~~~~~~~~~~~~ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:457:27: note: in expansion of macro 'MOZ_RELEASE_ASSERT' # define MOZ_ASSERT(...) MOZ_RELEASE_ASSERT(__VA_ARGS__) ^~~~~~~~~~~~~~~~~~ /root/firefox-gcc-last/js/src/frontend/TokenStream.h:1467:9: note: in expansion of macro 'MOZ_ASSERT' MOZ_ASSERT(lead == sourceUnits.previousCodeUnit(), ^~~~~~~~~~ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:417:71: error: template argument 1 is invalid static_assert(mozilla::detail::AssertionConditionType<decltype(x)>::isValid, \ ^ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:442:5: note: in expansion of macro 'MOZ_VALIDATE_ASSERT_CONDITION_TYPE' MOZ_VALIDATE_ASSERT_CONDITION_TYPE(expr); \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:450:39: note: in expansion of macro 'MOZ_ASSERT_HELPER2' #define MOZ_RELEASE_ASSERT_GLUE(a, b) a b ^ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:452:3: note: in expansion of macro 'MOZ_RELEASE_ASSERT_GLUE' MOZ_RELEASE_ASSERT_GLUE( \ ^~~~~~~~~~~~~~~~~~~~~~~ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/MacroArgs.h:16:26: note: in expansion of macro 'MOZ_CONCAT2' #define MOZ_CONCAT(x, y) MOZ_CONCAT2(x, y) ^~~~~~~~~~~ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/MacroArgs.h:73:51: note: in expansion of macro 'MOZ_CONCAT' #define MOZ_PASTE_PREFIX_AND_ARG_COUNT_GLUE(a, b) a b ^ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/MacroArgs.h:75:3: note: in expansion of macro 'MOZ_PASTE_PREFIX_AND_ARG_COUNT_GLUE' MOZ_PASTE_PREFIX_AND_ARG_COUNT_GLUE( \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:453:5: note: in expansion of macro 'MOZ_PASTE_PREFIX_AND_ARG_COUNT' MOZ_PASTE_PREFIX_AND_ARG_COUNT(MOZ_ASSERT_HELPER, __VA_ARGS__), \ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:457:27: note: in expansion of macro 'MOZ_RELEASE_ASSERT' # define MOZ_ASSERT(...) MOZ_RELEASE_ASSERT(__VA_ARGS__) ^~~~~~~~~~~~~~~~~~ /root/firefox-gcc-last/js/src/frontend/TokenStream.h:1467:9: note: in expansion of macro 'MOZ_ASSERT' MOZ_ASSERT(lead == sourceUnits.previousCodeUnit(), ^~~~~~~~~~
Assignee | ||
Comment 2•6 years ago
|
||
This is a very unsubtle gcc bug. It's complaining about a (.*)TokenStream(.*) class not having a previousCodeUnit member function, but the call very clearly is on |sourceUnits| which quite obviously is not a token stream class at all. Fortunately this looks like some interaction of |using| inside template classes that's trivially avoided by just throwing some |this-|-qualification on things. Will land that change shortly.
Flags: needinfo?(jwalden+bmo)
Assignee | ||
Comment 3•6 years ago
|
||
...okay, so comment 2 is technically correct, but the fix involves qualifying a massive number of spots in code. Enough that you basically should just remove a few |using|s for non-obvious reasons. I'm going to reduce a testcase and report a gcc bug. Maybe the gcc folks will have a suggestion for an elegant workaround, at least temporarily until gcc fixes this (IMO, potentially maybe even in existing releases). Barring that, I will probably be removing some |using|s.
Assignee | ||
Comment 4•6 years ago
|
||
I filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86379 with a mostly-reduced testcase that fails to compile with gcc 8.0+ but *does* compile with clang and with gcc<8.0. I don't know when they'll get back to us on what the best workaround is, if there is one. In the meantime, the |this|-qualifying approach works, so I'll clean up that patch and post it for review.
Assignee | ||
Comment 5•6 years ago
|
||
I'm so sorry. :-( Not all of these qualifications are necessary, but it isn't immediately clear to me *which* ones are necessary and *why*. *Because* it isn't clear, I think the best approach is to require that all accesses to |sourceUnits| and |charBuffer| be qualified, by removing the |using|s for each of them. Better to have one clear rule -- always |this->|-qualify -- than to leave a permanent minefield in place for future changes. (I know there's a distinction between the necessary/unnecessary ones because I first wrote a patch changing only the ones that caused compile errors, then removed the |using|s and added qualifiers for the rest.)
Attachment #8989301 -
Flags: review?(arai.unmht)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Comment 6•6 years ago
|
||
Comment on attachment 8989301 [details] [diff] [review] |this->|-qualify a bunch of function calls on |using Base::foo| members that gcc 8 completely chokes on and grossly misunderstands Review of attachment 8989301 [details] [diff] [review]: ----------------------------------------------------------------- Looks good :)
Attachment #8989301 -
Flags: review?(arai.unmht) → review+
Pushed by jwalden@mit.edu: https://hg.mozilla.org/integration/mozilla-inbound/rev/25d29385236c |this->|-qualify a bunch of function calls on |using Base::foo| members that gcc 8 completely chokes on and grossly misunderstands. r=arai
Reporter | ||
Updated•6 years ago
|
Comment 9•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/25d29385236c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Reporter | ||
Updated•6 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•