Closed
Bug 1418047
Opened 7 years ago
Closed 7 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: mozbugz, 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•7 years ago
|
||
We live in a C++14 world now.
Attachment #8929155 -
Flags: review?(core-build-config-reviews)
Comment 2•7 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•7 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•7 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•7 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•7 years ago
|
||
I confirm that I could compile bug 1329385 with the attached patch. Thanks Nathan.
Comment 7•7 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•7 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•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•