1444472 - error: default initialization of an object of const type 'const struct CheckHeaderToken' without a user-provided default constructor (nsCORSListenerProxy.cpp)

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P3)

Description

[Andrew McCreight [:mccr8]]

I'm seeing this in a local icecc build of Firefox, with clang. It is an opt build.

amccreight@bangles:/media/ssd/mc$ /usr/bin/clang --version
clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

My mozconfig is:

export CC="/usr/bin/clang"
export CXX="/usr/bin/clang++"

mk_add_options 'export CCACHE_PREFIX=icecc'
mk_add_options MOZ_MAKE_FLAGS="-j100"

mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-opt.noindex
mk_add_options AUTOCLOBBER=1

ac_add_options --enable-optimize
ac_add_options --disable-debug
ac_add_options --with-ccache
#ac_add_options --disable-ipdl-tests

#ac_add_options --disable-webrender

# Doesn't work with icecream.
#ac_add_options --enable-warnings-as-errors

Comment 1

[Andrew McCreight [:mccr8]]

The actual full error is:

 0:09.30 In file included from /media/ssd/mc/obj-opt.noindex/netwerk/protocol/http/Unified_cpp_protocol_http1.cpp:74:
 0:09.30 /media/ssd/mc/netwerk/protocol/http/nsCORSListenerProxy.cpp:230:5: error: default initialization of an object of const type 'const struct CheckHeaderToken' without a user-provided default constructor
 0:09.30   } checker;
 0:09.30     ^
 0:09.30            {}
 0:09.30 1 error generated.

Comment 2

[Andrew McCreight [:mccr8]]

GCC worked for me, FWIW.

Comment 3

[Michal Novotny [:michal]]

This code was added in bug 1443316. Nathan, can you please fix it?

Comment 4

[Nathan Froyd [:froydnj]]

Attachment: fix incorrect clang warning in nsCORSListenerProxy

Older versions of clang complain about this code, with:

netwerk/protocol/http/nsCORSListenerProxy.cpp:230:5: error: default initialization of an object of const type 'const struct CheckHeaderToken' without a user-provided default constructor

Later versions of the standard were amended to make this not an error,
but clang still warns.  Pacify clang by making the object not const.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fabb9cce9709
fix incorrect clang warning in nsCORSListenerProxy; r=valentin

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/fabb9cce9709

Comment 7

[:Felipe Gomes (needinfo for replies!)]

[Tracking Requested - why for this release]:

Could we uplift this to beta? Since 60 is going to be ESR, it would be nice to avoid this build error there.

Comment 8

[Nathan Froyd [:froydnj]]

Comment on attachment 8958208 [details] [diff] [review]
fix incorrect clang warning in nsCORSListenerProxy

(In reply to :Felipe Gomes (needinfo me!) from comment #7)
> [Tracking Requested - why for this release]:
> 
> Could we uplift this to beta? Since 60 is going to be ESR, it would be nice
> to avoid this build error there.

Sure, we could do that.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1443316
[User impact if declined]: None, strictly a packager/developer quality of life improvement.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes?
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Just fixing a compiler warning for certain versions of clang.
[String changes made/needed]: None.

Comment 9

[Julien Cristau [:jcristau]]

Comment on attachment 8958208 [details] [diff] [review]
fix incorrect clang warning in nsCORSListenerProxy

I guess that's harmless enough.  beta60+

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c90dadfb2b20

Comment 11

[Andrei Vaida [:avaida]]

(In reply to Nathan Froyd [:froydnj] from comment #8)
> [User impact if declined]: None, strictly a packager/developer quality of
> life improvement.
> [Is this code covered by automated tests?]: Yes.
> [Has the fix been verified in Nightly?]: Yes?
> [Needs manual test from QE? If yes, steps to reproduce]: No.

Setting qe-verify- based on Nathan's assessment on manual testing needs and the fact that this fix has automated coverage.