[PATCH] D133482: [LLVM] Fix GetErrcMessages.cmake module for WoA

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a reviewer: zero9178.
mstorsjo added a subscriber: zero9178.
mstorsjo added a comment.

I think @zero9178 was involved in adding this runtime check, adding him as reviewer too.

In D133482#3776741 <https://reviews.llvm.org/D133482#3776741>, @DavidSpickett wrote:

> I was curious about the reasoning from cmake here.
>
> https://cmake.org/cmake/help/latest/variable/CMAKE_TRY_COMPILE_CONFIGURATION.html#variable:CMAKE_TRY_COMPILE_CONFIGURATION:
>
>   Projects built by try_compile() and try_run() are built synchronously during the CMake configuration step. Therefore a specific build configuration must be chosen even if the generated build system supports multiple configurations.

I guess that this also means, that if you're building with such a multi configuration generator (e.g. the Visual Studio project file generator), there simply is no single `${CMAKE_BUILD_TYPE}` set at this point (or does LLVM's own logic set one anyway?). I think such build configurations are uncommon, but they probably still exist.

> Is it possibly better to set this at a higher level? I know there is some existing logic to error if you don't choose a build type for llvm now, maybe there?
>
> I think the overall idea of "you shouldn't need to be able to build debug if you chose to build release" is fine but there's probably more try_compiles.

Alternatively, maybe we should back up and reset this variable to what it was set to before? So that we only affect this particular one. Then again, (my cmake is a bit unclear on this point), does setting it here, within a function, mean it doesn't affect things outside of this function? Then it's probably safe as such...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133482/new/

https://reviews.llvm.org/D133482