-
Notifications
You must be signed in to change notification settings - Fork 17
fix(clang-cl): reduce warnings #900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
An automated preview of the documentation is available at https://900.mrdocs.prtest2.cppalliance.org/index.html |
$<$<CXX_COMPILER_ID:MSVC>:/MP> # multi-processor compilation | ||
/EHs # C++ Exception handling | ||
$<$<CONFIG:Debug>:/Oy-> # Disable frame pointer omission | ||
$<$<CXX_COMPILER_ID:Clang>:-Wno-unused-parameter> # ClangCL warning enabled with /W4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we fix the code triggering the warning instead of disabling the warning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a separate commit now. I initially added this because the warnings weren't present in the output of GCC and Clang on macOS/Linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. At some point we have to enable all these warnings there too. It's just that the project started without the these warnings and we always have higher priority stuff to work on.
@@ -192,7 +192,7 @@ class LegibleNames::Impl | |||
If the symbolhas no name, then a reserved name based | |||
on the type is returned instead. | |||
*/ | |||
std::string_view | |||
std::string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:)
An automated preview of the documentation is available at https://900.mrdocs.prtest2.cppalliance.org/index.html |
When compiling with clang-cl, there are warnings not present with MSVC
/MP
isn't a thing in clang-cl./W4
enables-Wunused-parameter
in clang-cl (or this might be enabled by default) - either way, it's not enabled in MSVC and generates some warnings here.getRawUnqualified
returned astd::string_view
to a temporarystd::string
and clang warned about it.-Wself-move
was ignored when__GNUC__
was defined. This isn't defined with clang-cl, so ignore it in the clang block.