[libc++] Option to specify a different compiler for testing

[Jake-Egan]

Summary

This patch gives the option to change the Lit test compiler by setting LIBCXX_TEST_COMPILER. Defaults to CMAKE_CXX_COMPILER if not specified.

[ldionne]

There are a few options here (including re-adding the old cxx_under_test lit param (perhaps under another name)). But first, I'd like to understand why you're not setting CMAKE_CXX_COMPILER to the compiler you're trying to run the tests with?

Requesting changes for review queue status.

[hubert.reinterpretcast]

In D137110#3899449, @ldionne wrote:

But first, I'd like to understand why you're not setting CMAKE_CXX_COMPILER to the compiler you're trying to run the tests with?

Is my understanding correct that changing CMAKE_CXX_COMPILER also changes the build compiler for the shared libraries?
The intention is to build using a "stable" compiler, but still allow testing with a compiler under development.

[phosek]

In D137110#3899667, @hubert.reinterpretcast wrote:

In D137110#3899449, @ldionne wrote:

But first, I'd like to understand why you're not setting CMAKE_CXX_COMPILER to the compiler you're trying to run the tests with?

Is my understanding correct that changing CMAKE_CXX_COMPILER also changes the build compiler for the shared libraries?
The intention is to build using a "stable" compiler, but still allow testing with a compiler under development.

What's the use case? I find this potentially problematic. What if the compiler specified for running tests with doesn't support the same set of flags as the one used for building?

[hubert.reinterpretcast]

In D137110#3899684, @phosek wrote:

What's the use case? I find this potentially problematic. What if the compiler specified for running tests with doesn't support the same set of flags as the one used for building?

The use case is to test a newer version of the compiler that is used to build. So the compiler specified for running tests should support at least the flags as the one used for building.

[Jake-Egan]

It looks like cxx_under_test would work for our use case. Is this option preferred?

[Jake-Egan]

If we'd like to go that route, I could update this patch to re-add cxx_under_test (or other name) as a lit parameter.

[philnik]

In D137110#3899667, @hubert.reinterpretcast wrote:

In D137110#3899449, @ldionne wrote:

But first, I'd like to understand why you're not setting CMAKE_CXX_COMPILER to the compiler you're trying to run the tests with?

Is my understanding correct that changing CMAKE_CXX_COMPILER also changes the build compiler for the shared libraries?
The intention is to build using a "stable" compiler, but still allow testing with a compiler under development.

Why can't you compile the library with the experimental compiler? If it can't properly compile that, it surely can't run the test suite.

[ldionne]

I think the preferred way forward here would be to make the libc++ test suite usable on its own, outside of LLVM. And then, you could use CMAKE_CXX_COMPILER to configure it, but you'd only be configuring the test suite, not the build of libc++ itself.

I think that would be extremely useful to other implementations as well, like libstdc++ and MSVC who want to run our test suite but don't care about building our runtimes.

[daltenty]

In D137110#3916097, @philnik wrote:

In D137110#3899667, @hubert.reinterpretcast wrote:

In D137110#3899449, @ldionne wrote:

But first, I'd like to understand why you're not setting CMAKE_CXX_COMPILER to the compiler you're trying to run the tests with?

Is my understanding correct that changing CMAKE_CXX_COMPILER also changes the build compiler for the shared libraries?
The intention is to build using a "stable" compiler, but still allow testing with a compiler under development.

Why can't you compile the library with the experimental compiler? If it can't properly compile that, it surely can't run the test suite.

That's simply not the same test though. In practice the same built shared library + headers can be consumed by multiple different compilers (which may encounter unique issues) when deployed. We'd like a hook to test such scenarios.

I think the preferred way forward here would be to make the libc++ test suite usable on its own, outside of LLVM. And then, you could use CMAKE_CXX_COMPILER to configure it, but you'd only be configuring the test suite, not the build of libc++ itself.

I think that would be extremely useful to other implementations as well, like libstdc++ and MSVC who want to run our test suite but don't care about building our runtimes.

Would we accomplish that by means of some flags we pass to the CMake config then? Essentially some options to say, skip building the library, just stand up the test hardness (maybe with an option to control whether the libc++ specific or just standard test are included). Very roughly, that would make configuring and invoking it something like this (assuming the library isn't the compiler search paths) IIUC:

$ cmake  -DCMAKE_CXX_COMPILER=/some/c++  -DLLVM_ENABLE_RUNTIMES=libcxx -DLIBCXX_TEST_ONLY=ON  -DLIBCXX_LIBRARY_DIR=/path/to/build/lib -DLIBCXX_HEADER_DIR=/path/to/build/include/c++/v1 -DLIBCXX_TEST_CONFIG="appropriate.cfg.in"
$ make check-cxx

[mstorsjo]

I'm not entirely sure what the best way would be to set things up to run the testsuite on its own, but I've been doing that manually, for running individual tests with third party C++ library implementations.

So far, I've made it by first taking a regular build+test setup, then making a copy of the lit site configs and llvm-lit scripts and modifying them to suit my use case. In my case, I test with the default C++ library shipped with a compiler, so I can mostly rip out most of what's set in %{compile_flags} and %{link_flags} (which serves to make the compiler use the just-built libc++ instead of the default one shipped with the compiler), but if you want to test specifically the just-built libc++ with a different compiler, you probably need fewer changes.

Anyway, my setup, for running libc++ tests against MSVC STL, looks like this:
A frontend shell script:

#!/bin/sh
# export PATH=/path/to/my/external/compiler:$PATH
export LIBCXX_SITE_CONFIG=$(cd $(dirname $0) && pwd)/msvc-lit.site.cfg
python3 $(dirname $0)/custom-lit "$@"

A custom copy of the llvm-lit script, named custom-lit: https://martin.st/temp/custom-lit
A handwritten site config, msvc-lit.site.cfg: https://martin.st/temp/msvc-lit.site.cfg
(The latter is mostly a hand-merged/set up version of cmake-bridge.cfg.in and one of the regular config files.)

Similarly I've got a set up for running tests against libstdc++.

libcxx/test/CMakeLists.txt

 if (NOT LIBCXX_ENABLE_RTTI)
   serialize_lit_param(enable_rtti False)
 endif()
 
 if (LIBCXX_ENABLE_ASSERTIONS)
   serialize_lit_param(enable_assertions True)
 endif()
 
+if (NOT LIBCXX_TEST_COMPILER)
+  set(LIBCXX_TEST_COMPILER "${CMAKE_CXX_COMPILER}" CACHE PATH
+    "Path to the Lit test compiler.")
+endif()
+
 if (CMAKE_CXX_COMPILER_TARGET)
   serialize_lit_param(target_triple "\"${CMAKE_CXX_COMPILER_TARGET}\"")
 else()
   serialize_lit_param(target_triple "\"${LLVM_DEFAULT_TARGET_TRIPLE}\"")
 endif()
 
 if (LLVM_USE_SANITIZER)
   serialize_lit_param(use_sanitizer "\"${LLVM_USE_SANITIZER}\"")

libcxx/test/configs/cmake-bridge.cfg.in

 config.name = os.path.basename('@LIBCXX_TEST_CONFIG@')
 config.test_source_root = os.path.join('@LIBCXX_SOURCE_DIR@', 'test')
 config.test_format = libcxx.test.format.CxxStandardLibraryTest()
 config.recursiveExpansionLimit = 10
 config.test_exec_root = os.path.join('@CMAKE_BINARY_DIR@', 'test')
 
 # Add substitutions for bootstrapping the test suite configuration
 import shlex
-config.substitutions.append(('%{cxx}', shlex.quote('@CMAKE_CXX_COMPILER@')))
+config.substitutions.append(('%{cxx}', shlex.quote('@LIBCXX_TEST_COMPILER@')))
 config.substitutions.append(('%{libcxx}', '@LIBCXX_SOURCE_DIR@'))
 config.substitutions.append(('%{include}', '@LIBCXX_GENERATED_INCLUDE_DIR@'))
 config.substitutions.append(('%{target-include}', '@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@'))
 config.substitutions.append(('%{lib}', '@LIBCXX_LIBRARY_DIR@'))
 config.substitutions.append(('%{executor}', '@LIBCXX_EXECUTOR@'))

libcxxabi/test/CMakeLists.txt

 endif()
 
 serialize_lit_param(enable_experimental False)
 
 if (LLVM_USE_SANITIZER)
   serialize_lit_param(use_sanitizer "\"${LLVM_USE_SANITIZER}\"")
 endif()
 
+if (NOT LIBCXX_TEST_COMPILER)
+  set(LIBCXX_TEST_COMPILER "${CMAKE_CXX_COMPILER}" CACHE PATH
+    "Path to the Lit test compiler.")
+endif()
+
 if (CMAKE_CXX_COMPILER_TARGET)
   serialize_lit_param(target_triple "\"${CMAKE_CXX_COMPILER_TARGET}\"")
 else()
   serialize_lit_param(target_triple "\"${LLVM_DEFAULT_TARGET_TRIPLE}\"")
 endif()
 
 foreach(param IN LISTS LIBCXXABI_TEST_PARAMS)
   string(REGEX REPLACE "(.+)=(.+)" "\\1" name "${param}")

libcxxabi/test/configs/cmake-bridge.cfg.in

 config.test_source_root = os.path.join('@LIBCXXABI_SOURCE_DIR@', 'test')
 config.test_format = libcxx.test.format.CxxStandardLibraryTest()
 config.recursiveExpansionLimit = 10
 config.test_exec_root = '@CMAKE_BINARY_DIR@'
 
 # TODO: This is a non-standard Lit attribute and we should have another way of accessing this.
 config.host_triple = '@LLVM_HOST_TRIPLE@'
 
-config.substitutions.append(('%{cxx}', '@CMAKE_CXX_COMPILER@'))
+config.substitutions.append(('%{cxx}', '@LIBCXX_TEST_COMPILER@'))
 config.substitutions.append(('%{libcxx}', '@LIBCXXABI_LIBCXX_PATH@'))
 config.substitutions.append(('%{include}', '@LIBCXXABI_SOURCE_DIR@/include'))
 config.substitutions.append(('%{cxx-include}', '@LIBCXXABI_HEADER_DIR@/include/c++/v1'))
 config.substitutions.append(('%{cxx-target-include}', '@LIBCXXABI_HEADER_DIR@/include/%{triple}/c++/v1'))
 config.substitutions.append(('%{lib}', '@LIBCXXABI_LIBRARY_DIR@'))
 config.substitutions.append(('%{executor}', '@LIBCXXABI_EXECUTOR@'))
 
 if @LIBCXXABI_USE_LLVM_UNWINDER@:
     config.substitutions.append(('%{maybe-include-libunwind}', '-I "@LIBCXXABI_LIBUNWIND_INCLUDES_INTERNAL@"'))
 else:
     config.substitutions.append(('%{maybe-include-libunwind}', ''))

libunwind/test/CMakeLists.txt

 endmacro()
 
 serialize_lit_param(enable_experimental False)
 
 if (LLVM_USE_SANITIZER)
   serialize_lit_param(use_sanitizer "\"${LLVM_USE_SANITIZER}\"")
 endif()
 
+if (NOT LIBCXX_TEST_COMPILER)
+  set(LIBCXX_TEST_COMPILER "${CMAKE_CXX_COMPILER}" CACHE PATH
+    "Path to the Lit test compiler.")
+endif()
+
 if (CMAKE_CXX_COMPILER_TARGET)
   serialize_lit_param(target_triple "\"${CMAKE_CXX_COMPILER_TARGET}\"")
 else()
   serialize_lit_param(target_triple "\"${LLVM_DEFAULT_TARGET_TRIPLE}\"")
 endif()
 
 foreach(param IN LISTS LIBUNWIND_TEST_PARAMS)
   string(REGEX REPLACE "(.+)=(.+)" "\\1" name "${param}")

libunwind/test/configs/cmake-bridge.cfg.in

 # Add a few features that are common to all the configurations
 if @LIBUNWIND_USES_ARM_EHABI@:
     config.available_features.add('libunwind-arm-ehabi')
 if not @LIBUNWIND_ENABLE_THREADS@:
     config.available_features.add('libunwind-no-threads')
 
 # Add substitutions for bootstrapping the test suite configuration
 import shlex
-config.substitutions.append(('%{cxx}', shlex.quote('@CMAKE_CXX_COMPILER@')))
+config.substitutions.append(('%{cxx}', shlex.quote('@LIBCXX_TEST_COMPILER@')))
 config.substitutions.append(('%{executor}', '@LIBUNWIND_EXECUTOR@'))
 config.substitutions.append(('%{include}', '@LIBUNWIND_SOURCE_DIR@/include'))
 config.substitutions.append(('%{lib}', '@LIBUNWIND_LIBRARY_DIR@'))