Closed Bug 1298472 Opened 3 years ago Closed 3 years ago

suppress even more warnings with clang-cl

Categories

(Firefox Build System :: General, defect)

All
Windows
defect
Not set

Tracking

(firefox51 fixed)

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(2 files)

We did this in bug 1232772 and we need to add a few more to cut down on warning spam during the build.
What are the warnings that you're getting?  If we're hitting warnings generated from the MSVC headers, those will probably need to be fixed upstream.  clang already turns off tons of warnings for system headers...

I'd prefer to not mask them en masse like this...
Comment on attachment 8785418 [details]
Bug 1298472 - part 1 - suppress more warnings in clang-cl;

https://reviewboard.mozilla.org/r/74614/#review73070

Waiting for an answer to ehsan's question.
Attachment #8785418 - Flags: review?(mh+mozilla)
(In reply to :Ehsan Akhgari from comment #2)
> What are the warnings that you're getting?  If we're hitting warnings
> generated from the MSVC headers, those will probably need to be fixed
> upstream.  clang already turns off tons of warnings for system headers...

-Wuser-defined-literals produces:

 2:03.11 C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE\string(668,15):  warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals]
 2:03.11 inline string operator "" s(const char *_Str, size_t _Len)
 2:03.11               ^
 2:03.11 C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE\string(673,16):  warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals]
 2:03.11 inline wstring operator "" s(const wchar_t *_Str, size_t _Len)
 2:03.11                ^
 2:03.11 C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE\string(678,18):  warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals]
 2:03.11 inline u16string operator "" s(const char16_t *_Str, size_t _Len)
 2:03.11                  ^
 2:03.11 C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE\string(683,18):  warning: user-defined literal suffixes not starting with '_' are reserved [-Wuser-defined-literals]
 2:03.11 inline u32string operator "" s(const char32_t *_Str, size_t _Len)
 2:03.11                  ^

-Wc++14-extensions actually probably isn't needed; I think that's a holdover from before I made clang-cl compile in c++14 mode.  So I guess we have one warning to report upstream?
Yes, sounds right!
OK, so the result of talking to upstream is that our STL header wrappers and clang's system header detection don't play nicely together: we include the full path to the STL headers, which doesn't properly identify them as system headers.  Fortunately, clang-cl supports #include_next, so we can just use that.

I'm also going to request that we suppress -Wignored-pragmas (we already suppress -Wunknown-pragmas), so people using #pragma intrinsic don't get a raft of warnings when using clang-cl.
Comment on attachment 8785418 [details]
Bug 1298472 - part 1 - suppress more warnings in clang-cl;

https://reviewboard.mozilla.org/r/74614/#review76886
Attachment #8785418 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8789838 [details]
Bug 1298472 - part 2 - use #include_next in STL header wrappers for clang-cl;

https://reviewboard.mozilla.org/r/77902/#review76888
Attachment #8789838 - Flags: review?(mh+mozilla) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dab403941f77
part 1 - suppress more warnings in clang-cl; r=glandium
https://hg.mozilla.org/integration/autoland/rev/99183448b819
part 2 - use #include_next in STL header wrappers for clang-cl; r=glandium
https://hg.mozilla.org/mozilla-central/rev/dab403941f77
https://hg.mozilla.org/mozilla-central/rev/99183448b819
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.