Page MenuHomePhabricator

[CMake] Replace LLVM_ENABLE_CXX1Y and friends with LLVM_CXX_STD
ClosedPublic

Authored by bogner on Apr 8 2019, 1:36 AM.

Details

Summary

Simplify building with particular C++ standards by replacing the
specific "enable standard X" flags with a flag that allows specifying
the standard you want directly.

We preserve compatibility with the existing flags so that anyone with
those flags in existing caches won't break mysteriously.

Diff Detail

Repository
rL LLVM

Event Timeline

bogner created this revision.Apr 8 2019, 1:36 AM
mehdi_amini accepted this revision.Apr 8 2019, 2:06 AM
This revision is now accepted and ready to land.Apr 8 2019, 2:06 AM
This revision was automatically updated to reflect the committed changes.
bogner added a comment.Apr 8 2019, 6:27 AM

Maybe, but that flag's behaviour is a bit complicated/surprising.

  • It uses gnu++ by default instead of c++, but it looks like we could set CXX_EXTENSIONS to no to disable that. This would mean that if someone wants to use gnu++ they'll need to know about the CXX_EXTENSIONS flag, of course.
  • Based on some testing, cmake doesn't really do the right thing with CMAKE_CXX_STANDARD very reliably. On cmake 3.12, at least, specifying 20 uses gnu++1z rather than gnu++2a like it should. It also seems to use gnu++1z rather than gnu++17 for 17 still, which is a bit odd but not the end of the world.
phosek added a subscriber: phosek.Apr 8 2019, 11:22 PM

This broke all our builders that use the LLVM runtimes build with the following error:

-- Performing Test CXX_SUPPORTS_CXX_STD
-- Performing Test CXX_SUPPORTS_CXX_STD - Failed
CMake Error at /b/s/w/ir/k/llvm-project/llvm/cmake/modules/HandleLLVMOptions.cmake:451 (message):
  The host compiler does not support '-std='.
Call Stack (most recent call first):
  CMakeLists.txt:111 (include)


-- Configuring incomplete, errors occurred!

The full log is here: https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8916679531057446976/+/steps/clang/0/steps/build/0/stdout

I suspect it's because we include HandleLLVMOptions at https://github.com/llvm/llvm-project/blob/master/llvm/runtimes/CMakeLists.txt#L111, but at that point we haven't yet compiled libc++, so any attempt to use is going to fail which is why this check is failing.

This broke all our builders that use the LLVM runtimes build with the following error:

-- Performing Test CXX_SUPPORTS_CXX_STD
-- Performing Test CXX_SUPPORTS_CXX_STD - Failed
CMake Error at /b/s/w/ir/k/llvm-project/llvm/cmake/modules/HandleLLVMOptions.cmake:451 (message):
  The host compiler does not support '-std='.
Call Stack (most recent call first):
  CMakeLists.txt:111 (include)


-- Configuring incomplete, errors occurred!

The full log is here: https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8916679531057446976/+/steps/clang/0/steps/build/0/stdout

I suspect it's because we include HandleLLVMOptions at https://github.com/llvm/llvm-project/blob/master/llvm/runtimes/CMakeLists.txt#L111, but at that point we haven't yet compiled libc++, so any attempt to use is going to fail which is why this check is failing.

I'm looking at the same failure in my cross-bootstrap of mingw as well.

The failure isn't due to not compiling libc++ yet, but when building individual runtime projects standalone outside of the llvm tree, HandleLLVMOptions.cmake is included in that cmake project, but not the llvm main CMake file. After this change, HandleLLVMOptions.cmake adds -std=${LLVM_CXX_STD} to the compiler options, but nothing has set ${LLVM_CXX_STD} yet, because the CMake file that sets it isn't included in standalone project builds.

bogner added a comment.Apr 9 2019, 1:15 AM

I moved where we set the default into HandleLLVMOptions in r357976, this should fix both issues. Sorry for the breakage!

I moved where we set the default into HandleLLVMOptions in r357976, this should fix both issues. Sorry for the breakage!

Thanks for fixing! That fix, together with rL357979, seems to have gotten things back in working order for me now.