1320656 - BitwiseCast(const From aFrom, To* aResult) syntax will change with C++17 and gcc is warning us

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Casting.h:42:1: error: mangled name for 'void mozilla::BitwiseCast(From, To*) [with To = void*; From = float (*)(float) throw ()]' will change in C++17 because the exception specification is part of a function type [-Werror=c++1z-compat]
 BitwiseCast(const From aFrom, To* aResult)
 ^~~~~~~~~~~
/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Casting.h:57:1: error: mangled name for 'To mozilla::BitwiseCast(From) [with To = void*; From = float (*)(float) throw ()]' will change in C++17 because the exception specification is part of a function type [-Werror=c++1z-compat]
 BitwiseCast(const From aFrom)
 ^~~~~~~~~~~

This is breaking the build with --enable-warnings-as-errors is used

Comment 1

[Jeff Walden [:Waldo]]

Is this literally an error in the header, or is it an error in some *use* of BitwiseCast?  The message suggests to me that it's the latter, in a use of BitwiseCast that explicitly parametrizes a BitwiseCast call with the From type.  If that's what it is, simply adding in |throw| to the explicitly-specified From (with whatever macro-guard is needed for backwards compatibility) should do the trick.

But it's impossible to say without more error context.  Mind providing it?

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Of course, sorry about that:
/usr/bin/g++-7 -std=gnu++11 -o Unified_cpp_js_src10.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 -DTRACING=1 -DENABLE_BINARYDATA -DENABLE_SIMD -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 -MD -MP -MF .deps/Unified_cpp_js_src10.o.pp  -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wc++14-compat -Wc++1z-compat -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe  -g -freorder-blocks -O3 -fno-omit-frame-pointer  -Werror -Wno-shadow -Werror=format -fdiagnostics-color  /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/js/src/Unified_cpp_js_src10.cpp
In file included from /root/firefox-gcc-last/js/src/jstypes.h:25:0,
                 from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/js/Utility.h:27,
                 from /root/firefox-gcc-last/js/src/jsalloc.h:18,
                 from /root/firefox-gcc-last/js/src/jsatom.h:12,
                 from /root/firefox-gcc-last/js/src/jsscript.h:18,
                 from /root/firefox-gcc-last/js/src/jit/BytecodeAnalysis.h:10,
                 from /root/firefox-gcc-last/js/src/jit/BytecodeAnalysis.cpp:7,
                 from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/js/src/Unified_cpp_js_src10.cpp:2:
/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Casting.h:42:1: error: mangled name for 'void mozilla::BitwiseCast(From, To*) [with To = void*; From = float (*)(float) throw ()]' will change in C++17 because the exception specification is part of a function type [-Werror=c++1z-compat]
 BitwiseCast(const From aFrom, To* aResult)
 ^~~~~~~~~~~
/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Casting.h:57:1: error: mangled name for 'To mozilla::BitwiseCast(From) [with To = void*; From = float (*)(float) throw ()]' will change in C++17 because the exception specification is part of a function type [-Werror=c++1z-compat]
 BitwiseCast(const From aFrom)

Comment 3

[Jeff Walden [:Waldo]]

Hmm, that doesn't identify the triggering code.  :-(  There aren't too many BitwiseCast calls in js/, tho.  Skimming them, I don't see any that *explicitly* pass in float(*)(float) as From-type.  So my guess is that this is what's responsible:

/home/jwalden/moz/slots/js/src/jstypes.h
208:#define JS_FUNC_TO_DATA_PTR(type, fun)  (mozilla::BitwiseCast<type>(fun))
209:#define JS_DATA_TO_FUNC_PTR(type, ptr)  (mozilla::BitwiseCast<type>(ptr))

and if we inserted in second template parameters of |decltype(&fun)| and |decltype(ptr)| that might be a fix.  But it's hard to say without testing locally.  Maybe I need to spin up a new gcc build again...

Comment 5

[Sylvestre Ledru [:Sylvestre]]

gcc renamed the warning to noexcept-type:
/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Casting.h:42:1: error: mangled name for 'void mozilla::BitwiseCast(From, To*) [with To = void*; From = float (*)(float) throw ()]' will change in C++17 because the exception specification is part of a function type [-Werror=noexcept-type]

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Attachment: bug 1320656 - When exists, add -Wno-noexcept-type to the js build flags until we have a proper fix

Review commit: https://reviewboard.mozilla.org/r/164882/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164882/

Comment 7

[Sylvestre Ledru [:Sylvestre]]

As I don't know how to fix it (and it is only when we will switch to C++17), I am proposing to disable the failure when we encounter this warning.

Comment 8

[Mike Hommey [:glandium]]

It seems like this should be fixed by replacing throw() everywhere it's used with a macro that expands to either throw() or noexcept(true) depending on the value of __cplusplus, similar to bug 1383919.

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8893783 [details]
bug 1320656 - When exists, add -Wno-noexcept-type to the js build flags until we have a proper fix

https://reviewboard.mozilla.org/r/164882/#review172130

::: js/src/moz.build:737
(Diff revision 1)
>  
>  if CONFIG['GNU_CXX']:
>      # Also disable strict-aliasing for GCC compiler, that is enabled by default
>      # starting with version 7.1, see Bug 1363009
> -    CXXFLAGS += ['-Wno-shadow', '-Werror=format', '-fno-strict-aliasing']
> +    CXXFLAGS += ['-Wno-shadow', '-Werror=format', '-fno-strict-aliasing',
> +                 '-Wno-error=noexcept-type']

Add a comment pointing to this bug.

Also, a note about what the warning is would help relativize. Per https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html:

  "Warn if the C++1z feature making noexcept part of a function type changes the mangled name of a symbol relative to C++14. Enabled by -Wabi and -Wc++1z-compat."

Unless that's exposed as a public ABI, this is pretty much "so what?"

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8893783 [details]
bug 1320656 - When exists, add -Wno-noexcept-type to the js build flags until we have a proper fix

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164882/diff/1-2/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/61cf451831f5
Disable the failure on -Wnoexcept-type until we have a proper fix r=glandium

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for bustage in RegExp.o:

https://hg.mozilla.org/integration/autoland/rev/03bdb029f9c6a82c7bbc25c25ebdc87e240d8f94

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=61cf451831f5673de926d67d74d9e03f5cbb642d&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable
Build log: https://treeherder.mozilla.org/logviewer.html#?job_id=122213998&repo=autoland

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Fails because of:
[task 2017-08-10T07:59:39.784045Z] 07:59:39     INFO -  cc1plus: error: -Werror=noexcept-type: no option -Wnoexcept-type
I guess replacing it by -Wno-noexcept-type  could work...

Comment 14

[Mike Hommey [:glandium]]

I bet not. This is likely a new flag that is not supported by GCC 4.9.

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8893783 [details]
bug 1320656 - When exists, add -Wno-noexcept-type to the js build flags until we have a proper fix

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164882/diff/2-3/

Comment 16

[Sylvestre Ledru [:Sylvestre]]

As you expected, cc1plus: error: unrecognized command line option "-Wno-noexcept-type" [-Werror]
I will add the detection of the flag.

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8893783 [details]
bug 1320656 - When exists, add -Wno-noexcept-type to the js build flags until we have a proper fix

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164882/diff/3-4/

Comment 18

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8893783 [details]
bug 1320656 - When exists, add -Wno-noexcept-type to the js build flags until we have a proper fix

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164882/diff/4-5/

Comment 19

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8893783 [details]
bug 1320656 - When exists, add -Wno-noexcept-type to the js build flags until we have a proper fix

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164882/diff/5-6/

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8893783 [details]
bug 1320656 - When exists, add -Wno-noexcept-type to the js build flags until we have a proper fix

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/164882/diff/6-7/

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/67963f1a665a
When exists, add -Wno-noexcept-type to the js build flags until we have a proper fix r=glandium

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/67963f1a665a

Comment 23

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:froydnj, maybe it's time to close this bug?

Comment 24

[Nathan Froyd [:froydnj]]

I guess we've fixed this, and we can open a new bug for the proper fix referenced in comment 21?  (Also, it amuses me to redirect sylvestre's bot inquires to sylvestre ;)

Comment 25

[Sylvestre Ledru [:Sylvestre]]

Yeah, Irony, and I reported it! :p