Closed
Bug 1418047
Opened 3 years ago
Closed 3 years ago
Cannot compile C++14 code yet, because of -Wc++-11-compat
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox59 fixed)
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: gerald, Assigned: froydnj)
References
Details
Attachments
(1 file)
970 bytes,
patch
|
rillian
:
review+
|
Details | Diff | Splinter Review |
Follow-up to bug 1325632. Gerald Squelart [:gerald] from bug 1325632 comment #45: I tried compiling my long-pending patch (which requires C++ 14) for bug 1329385, and I got: > gmake[4]: Entering directory 'obj-mozilla-central/dom/media/gmp' > In file included from obj-mozilla-central/dom/media/gmp/Unified_cpp_dom_media_gmp1.cpp:38: > dom/media/gmp/GMPServiceParent.cpp:378:45: > error: initialized lambda captures are incompatible with C++ standards before C++14 [-Werror,-Wc++98-c++11-compat] > [self, tags, api, nodeIdString, helper, holder(Move(holder))] > ^ > 1 error generated. Compiler invocation: > /usr/bin/clang++ -std=gnu++14 -o Unified_cpp_dom_media_gmp1.o -c -fvisibility=hidden -fvisibility-inlines-hidden > -DDEBUG=1 -DGMP_SAFE_SHMEM -DOS_POSIX=1 -DOS_MACOSX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE > -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I[...] -fPIC -DMOZILLA_CLIENT -include obj-mozilla-central/mozilla-config.h > -Qunused-arguments -D_FORTIFY_SOURCE=2 -Qunused-arguments -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers > -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return > -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis -Wc++14-compat -Wc++14-compat-pedantic > -Wc++1z-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion > -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat > -Wno-gnu-zero-variadic-macro-arguments -Wformat-security -Wno-unknown-warning-option -Wno-return-type-c-linkage > -fno-sized-deallocation -D_FORTIFY_SOURCE=2 -fno-exceptions -fno-strict-aliasing -stdlib=libc++ -fno-rtti > -fno-exceptions -fno-math-errno -pthread -pipe -g -fno-omit-frame-pointer -Werror -Wno-error=shadow -MD -MP -MF > .deps/Unified_cpp_dom_media_gmp1.o.pp -fcolor-diagnostics > obj-mozilla-central/dom/media/gmp/Unified_cpp_dom_media_gmp1.cpp Mac 10.12.6, Apple LLVM version 8.1.0 (clang-802.0.42) Nathan Froyd [:froydnj] from bug 1325632 comment #46: > That's not expected...but I think we should have removed -Wc++-11-compat > from the warnings flags as well.
![]() |
Assignee | |
Comment 1•3 years ago
|
||
We live in a C++14 world now.
Attachment #8929155 -
Flags: review?(core-build-config-reviews)
Comment 2•3 years ago
|
||
Comment on attachment 8929155 [details] [diff] [review] remove -Wc++-compat warning Review of attachment 8929155 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/moz.configure/warnings.configure @@ -19,5 @@ > # lots of useful warnings > add_gcc_warning('-Wall') > > -# catches C++ version forward-compat issues > -add_gcc_warning('-Wc++11-compat', cxx_compiler) Is there a -Wc++14-compat warning we should use instead?
Attachment #8929155 -
Flags: review?(core-build-config-reviews) → review+
Comment 3•3 years ago
|
||
Comment on attachment 8929155 [details] [diff] [review] remove -Wc++-compat warning Review of attachment 8929155 [details] [diff] [review]: ----------------------------------------------------------------- Yep. I assume this still makes sense with all the `std=c++11` I see in the tree in particular directories?
![]() |
Assignee | |
Comment 4•3 years ago
|
||
(In reply to Ralph Giles (:rillian) | needinfo me from comment #2) > ::: build/moz.configure/warnings.configure > @@ -19,5 @@ > > # lots of useful warnings > > add_gcc_warning('-Wall') > > > > -# catches C++ version forward-compat issues > > -add_gcc_warning('-Wc++11-compat', cxx_compiler) > > Is there a -Wc++14-compat warning we should use instead? We already have that warning enabled, curiously enough!
![]() |
Assignee | |
Comment 5•3 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #3) > Yep. I assume this still makes sense with all the `std=c++11` I see in the > tree in particular directories? Uh. I think so? Assuming those directories aren't going to be using C++14 features. Are all these directories third-party directories?
Assignee: nobody → nfroyd
Flags: needinfo?(nalexander)
Reporter | ||
Comment 6•3 years ago
|
||
I confirm that I could compile bug 1329385 with the attached patch. Thanks Nathan.
Comment 7•3 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #5) > (In reply to Nick Alexander :nalexander from comment #3) > > Yep. I assume this still makes sense with all the `std=c++11` I see in the > > tree in particular directories? > > Uh. I think so? Assuming those directories aren't going to be using C++14 > features. Are all these directories third-party directories? The js/ and browser/extensions/mortar/ bits of https://searchfox.org/mozilla-central/search?q=std%3Dc%2B%2B11&path= don't look super third-party.
Flags: needinfo?(nalexander)
Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/10f07d496d6a remove -Wc++11-compat warning; r=rillian
Comment 9•3 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #4) > > Is there a -Wc++14-compat warning we should use instead? > > We already have that warning enabled, curiously enough! We should probably remove -Wc++14-compat because it warns about code whose meaning differs between C++11 and C++14. We want the new C++14 meanings, so we don't need to warn about the changes. We do still -Wc++1z-compat/etc because want to know about C++14 code we're writing whose behavior will change in C++17.
Comment 10•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/10f07d496d6a
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Updated•3 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•