Closed
Bug 1232772
Opened 8 years ago
Closed 8 years ago
suppress numerous clang-style warnings when using clang-cl
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox46 fixed)
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: froydnj, Assigned: froydnj)
References
Details
Attachments
(1 file, 1 obsolete file)
8.07 KB,
patch
|
glandium
:
review+
glandium
:
feedback+
|
Details | Diff | Splinter Review |
Suppressing these warnings significantly cuts down the warning spam on clang-cl builds.
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8698577 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Comment 2•8 years ago
|
||
I guess we should add these to js/src/configure.in too...
Assignee | ||
Comment 3•8 years ago
|
||
Now with js/src/ modifications.
Attachment #8698586 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•8 years ago
|
Attachment #8698577 -
Attachment is obsolete: true
Attachment #8698577 -
Flags: review?(mh+mozilla)
Comment 4•8 years ago
|
||
Comment on attachment 8698586 [details] [diff] [review] suppress numerous clang-style warnings when using clang-cl Review of attachment 8698586 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +2287,5 @@ > + CFLAGS="$CFLAGS -Wno-invalid-noreturn" > + CXXFLAGS="$CXXFLAGS -Wno-invalid-noreturn" > + # Missing |override| on virtual function declarations isn't > + # something that MSVC currently warns about. > + CXXFLAGS="$CXXFLAGS -Wno-inconsistent-missing-override" Is that something we're hitting on system headers or windows platform code? Because IIRC, we're supposed to use override consistently in Gecko code. @@ +2290,5 @@ > + # something that MSVC currently warns about. > + CXXFLAGS="$CXXFLAGS -Wno-inconsistent-missing-override" > + # We declare |operator delete(void*)| differently than MSVC, > + # which causes a raft of warnings if we leave this enabled. > + CXXFLAGS="$CXXFLAGS -Wno-implicit-exception-spec-mismatch" Can you provide more detail here, because this might actually be important.
Attachment #8698586 -
Flags: review?(mh+mozilla) → feedback+
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #4) > Comment on attachment 8698586 [details] [diff] [review] > suppress numerous clang-style warnings when using clang-cl > > Review of attachment 8698586 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: configure.in > @@ +2287,5 @@ > > + CFLAGS="$CFLAGS -Wno-invalid-noreturn" > > + CXXFLAGS="$CXXFLAGS -Wno-invalid-noreturn" > > + # Missing |override| on virtual function declarations isn't > > + # something that MSVC currently warns about. > > + CXXFLAGS="$CXXFLAGS -Wno-inconsistent-missing-override" > > Is that something we're hitting on system headers or windows platform code? > Because IIRC, we're supposed to use override consistently in Gecko code. These appear to be coming from Gecko graphics headers and Gecko accessibility headers. Some of them are fixable; some of them come from macros defined by the Windows IDL compiler, so are not easily fixable. We could fix all the warnings, but they're just going to come back, seeing as how nothing is checking for this with our normal Windows builds... > @@ +2290,5 @@ > > + # something that MSVC currently warns about. > > + CXXFLAGS="$CXXFLAGS -Wno-inconsistent-missing-override" > > + # We declare |operator delete(void*)| differently than MSVC, > > + # which causes a raft of warnings if we leave this enabled. > > + CXXFLAGS="$CXXFLAGS -Wno-implicit-exception-spec-mismatch" > > Can you provide more detail here, because this might actually be important. The declaration from <new> says: void __CRTDECL operator delete(void *) _THROW0(); and the warning says: function previously declared with an explicit exception specification redeclared with an implicit exception specification Hm. I think that means our clang isn't turning off exceptions, because <xstddef> only defines THROW0 to something if _HAS_EXCEPTIONS. Maybe that's not a good thing...
Assignee | ||
Comment 6•8 years ago
|
||
There's also a lot of inconsistent override spam from webrtc, which is just annoying.
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #5) > > @@ +2290,5 @@ > > > + # something that MSVC currently warns about. > > > + CXXFLAGS="$CXXFLAGS -Wno-inconsistent-missing-override" > > > + # We declare |operator delete(void*)| differently than MSVC, > > > + # which causes a raft of warnings if we leave this enabled. > > > + CXXFLAGS="$CXXFLAGS -Wno-implicit-exception-spec-mismatch" > > > > Can you provide more detail here, because this might actually be important. > > The declaration from <new> says: > > void __CRTDECL operator delete(void *) _THROW0(); > > and the warning says: > > function previously declared with an explicit exception specification > redeclared with an implicit exception specification > > Hm. I think that means our clang isn't turning off exceptions, because > <xstddef> only defines THROW0 to something if _HAS_EXCEPTIONS. Maybe that's > not a good thing... I don't completely understand this warning now, because we *are* passing -D_HAS_EXCEPTIONS=0, so there's no explicit exception specification on the function. Unless clang is defining |operator delete| internally and then complaining that MSVC's headers (not ours) redeclare it improperly? In which case I don't think there's anything we can do here, unless clang-cl supports -fno-exceptions?
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3d2670d0cc5e
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Assignee | ||
Comment 10•8 years ago
|
||
Sigh, apparently mistyped the bug number for a bustage fix. Mike, WDYT about comment 5 and following?
Status: RESOLVED → REOPENED
Flags: needinfo?(mh+mozilla)
Resolution: FIXED → ---
Comment 11•8 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #7) > I don't completely understand this warning now, because we *are* passing > -D_HAS_EXCEPTIONS=0, so there's no explicit exception specification on the > function. Unless clang is defining |operator delete| internally and then > complaining that MSVC's headers (not ours) redeclare it improperly? In > which case I don't think there's anything we can do here, unless clang-cl > supports -fno-exceptions? Could you check what a .i looks like for one of the files where the warning happens?
Flags: needinfo?(mh+mozilla)
Assignee | ||
Comment 12•8 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #11) > Could you check what a .i looks like for one of the files where the warning > happens? Sure. delete is declared via <new> as: |void __cdecl operator delete(void *) ;| (None of the |delete| or |new| variants have a throwing declaration on them)
Flags: needinfo?(mh+mozilla)
Comment 13•8 years ago
|
||
Comment on attachment 8698586 [details] [diff] [review] suppress numerous clang-style warnings when using clang-cl Review of attachment 8698586 [details] [diff] [review]: ----------------------------------------------------------------- Considering comment 12, let's go for it.
Attachment #8698586 -
Flags: review+
Updated•8 years ago
|
Flags: needinfo?(mh+mozilla)
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0912f477bd73
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•