1418047 - Cannot compile C++14 code yet, because of -Wc++-11-compat

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gerald Squelart [:gerald] (he/him)]

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.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: remove -Wc++-compat warning

We live in a C++14 world now.

Comment 2

[Ralph Giles (:rillian)]

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?

Comment 3

[Nick Alexander :nalexander [he/him]]

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?

Comment 4

[Nathan Froyd [:froydnj]]

(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!

Comment 5

[Nathan Froyd [:froydnj]]

(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?

Comment 6

[Gerald Squelart [:gerald] (he/him)]

I confirm that I could compile bug 1329385 with the attached patch. Thanks Nathan.

Comment 7

[Nick Alexander :nalexander [he/him]]

(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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/10f07d496d6a
remove -Wc++11-compat warning; r=rillian

Comment 9

[Chris Peterson [:cpeterson]]

(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

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/10f07d496d6a