[PATCH] D155431: [CMake] Clean up old code for handling MSVC runtime setting the old way

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: llvm/cmake/modules/ChooseMSVCCRT.cmake:12-13
+# setting different choices for different build configurations (for
+# multi-config generators), but translating multiple differing choices to
+# CMAKE_MSVC_RUNTIME_LIBRARY isn't supported.
 
----------------
mstorsjo wrote:
> smeenai wrote:
> > Does using generator expressions for this not work, as suggested in https://cmake.org/cmake/help/latest/variable/CMAKE_MSVC_RUNTIME_LIBRARY.html?
> I haven't tested myself, but I would believe them to work.
> 
> What I'm trying to say here, is that I'm not trying to make a generic function that reads a number of `LLVM_USE_CRT_*` options and generates one single generator expression for `CMAKE_MSVC_RUNTIME_LIBRARY` that would express exactly the chosen combination.
> 
> I.e. for trivial cases with `LLVM_USE_CRT_*` this should help you along and you'd get a kind warning asking you to move onwards to `CMAKE_MSVC_RUNTIME_LIBRARY` at some point. If you have a nontrivial case, you'll to express it with a generator expression directly in `CMAKE_MSVC_RUNTIME_LIBRARY` right away.
> 
> I.e. "isn't supported" refers to the transitionary implementation here, not about what `CMAKE_MSVC_RUNTIME_LIBRARY` can do.
> 
> I guess I could reword it to be clearer.
Ah, got it, that makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155431