This extension point is not needed. Provide the equivalent option LLVM_ENABLE_CXX1Z which mirrors the previous extension point. Rely on CMake to provide the check for the compiler instead.
Diff Detail
- Repository
- rL LLVM
Event Timeline
CMakeLists.txt | ||
---|---|---|
57 | Should be 14 no? |
Why do you want LLVM_CXX_STD to be removed? I suppose somebody will eventually add LLVM_ENABLE_CXX2A, which will require additional logic to ensure that somebody doesn't set LLVM_ENABLE_CXX2A and LLVM_ENABLE_CXX1Z at the same time. I think this can be avoided by removing LLVM_ENABLE_CXX1Z entirely and going through LLVM_CXX_STD entirely.
Can we avoid having any custom variables for this at all and just use CMAKE_CXX_STANDARD? People can do -DCMAKE_CXX_STANDARD=17 or whatever else they want.
My reasoning behind that was that it seemed that some people were concerned about cached variables not being easy to remove in subsequent updates. This will now default to 14, but allow the user to specify a different value.
The change itself looks fine, but idk if we need to preserve support for any of the older variables (LLVM_ENABLE_CXX1Z in particular). Idk if there's an easy way to e.g. audit bot configurations to see what variables they're using ... maybe posting to llvm-dev would be a good idea?
Everything about this change seems good to me, the only thing I'd suggest is that we add a documentation note about CMAKE_CXX_STANDARD to docs/CMake.rst.
Should be 14 no?