Avoid errors due to "default label in switch which covers all enumeration values" in Windows codepath #1302
Conversation
…tion values" in Windows codepath This applies a fix that used to exist in LLVM's downstream copy of this library, from llvm/llvm-project@948ce4e. I presume this warning isn't present if built with MSVC or Clang-cl, but it's printed in MinGW mode. As the benchmark library adds -Werror, this is a fatal error when builtin MinGW mode.
|
thanks! |
|
(Based on mstorsjo's comment in https://reviews.llvm.org/D112012) Would we want to soften -Werror in |
|
i prefer not to soften it. i rather like that we have a strict build requirement. |
Would you consider softening it in the case when benchmark is a subcomponent as part of a larger build? (I.e. cmake root for isn’t equal to the current directory.) Alternatively a way to opt out of the behavior? I appreciate the desire to keep it strict when focusing on benchmark itself, but when pulled in as a dependency of something else, |
This reapplies a fix from 948ce4e, whichn't originally submitted upstream. I has now been merged upstream though, in google/benchmark#1302. When benchmarks were unified in 5dda2ef, it lost this change, but it also lost another local modification, where benchmark's CMakeLists.txt was modified to comment out adding -Werror. (This change was part of the original import in 0addd17.) As the benchmark library is built automatically by default, when building all of LLVM (contrary to the copy in libcxx, which wasn't built by default), building it with -Werror by default is very brittle. This fixes building LLVM with MinGW. (It wasn't broken in MSVC mode, as the benchmark library doesn't add -Werror or anything equivalent in MSVC mode, and it's unclear if this warning is enabled in that mode at all.) Differential Revision: https://reviews.llvm.org/D115434
|
there's a weird virality here, i think. if benchmark is included as a subproject, we have to adopt the least strict options of all projects that include it. but the opposite is true, that the parent project has to adopt the most strict of its subprojects. ideally, imo, a subproject should be built as a separate unit and linked in to the parent project build. building everything all-in-one just causes these sorts of issues. |
|
We already have |
|
😂 .. the project is moving too fast for me to keep track of. @mstorsjo does this help? 😁 |
Oh, indeed, that does help. That's not available in the old copy of benchmark present in LLVM, but I guess one could suggest backporting 285e5e9 to LLVM's copy - or just update the whole library to the latest version. |
|
updating llvm to the latest sounds like a good idea anyway given the constant stream of changes/improvements. |
Yeah, that's of course the ideal. But doing that might end up exposing other issues (which of course would be great to fix and sort out) which would be better handled by someone with more time to look into it... |
|
I'll do an update to latest, and also then pre-set BENCHMARK_ENABLE_WERROR off. |
Thanks, that'd be appreciated! |
|
Waiting for #1305, which should unblock refreshing the llvm copy. |
…tion values" in Windows codepath (google#1302) This applies a fix that used to exist in LLVM's downstream copy of this library, from llvm/llvm-project@948ce4e. I presume this warning isn't present if built with MSVC or Clang-cl, but it's printed in MinGW mode. As the benchmark library adds -Werror, this is a fatal error when builtin MinGW mode.
This applies a fix that used to exist in LLVM's downstream copy of
this library, from
llvm/llvm-project@948ce4e.
I presume this warning isn't present if built with MSVC or Clang-cl,
but it's printed in MinGW mode. As the benchmark library adds
-Werror, this is a fatal error when builtin MinGW mode.