[libcxx] [test] Fix testing on windows with c++experimental enabled

[mstorsjo]

Summary

The straightforward AddLinkFlag('-lc++experimental') approach doesn't
work on e.g. MSVC; instead use logic similar to how libc++ itself
is linked, but with the exception that libc++experimental always is
linked statically.

[mstorsjo]

Sorry, this one doesn't work as is; the link flags substitutions are already set up when libcxx.test.newconfig.configure returns, so it's too late to add anything to it after that...

[mstorsjo]

As it's too late to update link_flags after running newconfig.configure, add a simplified form of conditional logic in a lambda instead. This lambda requires knowledge about what kind of library naming/linking form is going to be used though, which is (not nicely) passed via a flag in available_features.

[ldionne]

Can we try detecting MSVC this way instead:

diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index e54cecef0ab8..2fb7e003ce03 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -14,6 +14,8 @@ import sys
 _isClang      = lambda cfg: '__clang__' in compilerMacros(cfg) and '__apple_build_version__' not in compilerMacros(cfg)
 _isAppleClang = lambda cfg: '__apple_build_version__' in compilerMacros(cfg)
 _isGCC        = lambda cfg: '__GNUC__' in compilerMacros(cfg) and '__clang__' not in compilerMacros(cfg)
+_isMSVC       = lambda cfg: '_MSC_VER' in compilerMacros(cfg)
+_msvcVersion  = lambda cfg: (int(compilerMacros(cfg)['_MSC_VER']) // 100, int(compilerMacros(cfg)['_MSC_VER']) % 100)
 
 DEFAULT_FEATURES = [
   Feature(name='fcoroutines-ts',
@@ -81,6 +83,10 @@ DEFAULT_FEATURES = [
   Feature(name=lambda cfg: 'gcc-{__GNUC__}'.format(**compilerMacros(cfg)),                                                         when=_isGCC),
   Feature(name=lambda cfg: 'gcc-{__GNUC__}.{__GNUC_MINOR__}'.format(**compilerMacros(cfg)),                                        when=_isGCC),
   Feature(name=lambda cfg: 'gcc-{__GNUC__}.{__GNUC_MINOR__}.{__GNUC_PATCHLEVEL__}'.format(**compilerMacros(cfg)),                  when=_isGCC),
+
+  Feature(name='msvc',                                                                                                             when=_isMSVC),
+  Feature(name=lambda cfg: 'msvc-{}'.format(*_msvcVersion(cfg)),                                                                   when=_isMSVC),
+  Feature(name=lambda cfg: 'msvc-{}.{}'.format(*_msvcVersion(cfg)),                                                                when=_isMSVC),
 ]
 
 # Deduce and add the test features that that are implied by the #defines in

I'm sure there's stuff to fix with that, but basically I'd rather add a new feature to detect msvc correctly all the time than hardcode it based on windows.

[mstorsjo]

In D99177#2644842, @ldionne wrote:

Can we try detecting MSVC this way instead:

diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index e54cecef0ab8..2fb7e003ce03 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -14,6 +14,8 @@ import sys
 _isClang      = lambda cfg: '__clang__' in compilerMacros(cfg) and '__apple_build_version__' not in compilerMacros(cfg)
 _isAppleClang = lambda cfg: '__apple_build_version__' in compilerMacros(cfg)
 _isGCC        = lambda cfg: '__GNUC__' in compilerMacros(cfg) and '__clang__' not in compilerMacros(cfg)
+_isMSVC       = lambda cfg: '_MSC_VER' in compilerMacros(cfg)
+_msvcVersion  = lambda cfg: (int(compilerMacros(cfg)['_MSC_VER']) // 100, int(compilerMacros(cfg)['_MSC_VER']) % 100)
 
 DEFAULT_FEATURES = [
   Feature(name='fcoroutines-ts',
@@ -81,6 +83,10 @@ DEFAULT_FEATURES = [
   Feature(name=lambda cfg: 'gcc-{__GNUC__}'.format(**compilerMacros(cfg)),                                                         when=_isGCC),
   Feature(name=lambda cfg: 'gcc-{__GNUC__}.{__GNUC_MINOR__}'.format(**compilerMacros(cfg)),                                        when=_isGCC),
   Feature(name=lambda cfg: 'gcc-{__GNUC__}.{__GNUC_MINOR__}.{__GNUC_PATCHLEVEL__}'.format(**compilerMacros(cfg)),                  when=_isGCC),
+
+  Feature(name='msvc',                                                                                                             when=_isMSVC),
+  Feature(name=lambda cfg: 'msvc-{}'.format(*_msvcVersion(cfg)),                                                                   when=_isMSVC),
+  Feature(name=lambda cfg: 'msvc-{}.{}'.format(*_msvcVersion(cfg)),                                                                when=_isMSVC),
 ]
 
 # Deduce and add the test features that that are implied by the #defines in

I'm sure there's stuff to fix with that, but basically I'd rather add a new feature to detect msvc correctly all the time than hardcode it based on windows.

Thanks, I'll have a try with that to see if it works. As all parameters are evaluated before all features (in newconfig.configure), would these be set already at the point when we need them when handling a parameter?

[mstorsjo]

Add a _isMSVC helper function in features.py and add a 'msvc' feature via that.
As the features themselves are processed after processing parameters, we can't check for the feature itself, but use the _isMSVC function.

[ldionne]

LGTM, but please consider populating the version.

libcxx/utils/libcxx/test/features.py

87

Do we want to also populate the MSVC version like we do for GCC and Clang?

[mstorsjo]

libcxx/utils/libcxx/test/features.py

87

Sure, I can do that for consistency.

In cases when testing with libc++, it's actually Clang impersonating MSVC - but the version of MSVC that Clang tries to impersonate can still be useful to know. (Clang has to emulate certain MSVC bugs depending on the emulated MSVC version at times. And in some cases, whether Clang correctly emulates newer MSVC features, or stopping emulating old MSVC bugs, depends on the Clang version as well.) So in practice, using the version for anything can be tricky/messy, but I can add it in any case.

libcxx/utils/libcxx/test/features.py

 #===----------------------------------------------------------------------===##
 #
 # Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 #===----------------------------------------------------------------------===##
 
 from libcxx.test.dsl import *
 import re
 import shutil
 import sys
 
 _isClang      = lambda cfg: '__clang__' in compilerMacros(cfg) and '__apple_build_version__' not in compilerMacros(cfg)
 _isAppleClang = lambda cfg: '__apple_build_version__' in compilerMacros(cfg)
 _isGCC        = lambda cfg: '__GNUC__' in compilerMacros(cfg) and '__clang__' not in compilerMacros(cfg)
+_isMSVC       = lambda cfg: '_MSC_VER' in compilerMacros(cfg)
+_msvcVersion  = lambda cfg: (int(compilerMacros(cfg)['_MSC_VER']) // 100, int(compilerMacros(cfg)['_MSC_VER']) % 100)
 
 DEFAULT_FEATURES = [
   Feature(name='fcoroutines-ts',
           when=lambda cfg: hasCompileFlag(cfg, '-fcoroutines-ts') and
                            featureTestMacros(cfg, flags='-fcoroutines-ts').get('__cpp_coroutines', 0) >= 201703,
           actions=[AddCompileFlag('-fcoroutines-ts')]),
 
   Feature(name='thread-safety',
DEFAULT_FEATURES = [
   Feature(name=lambda cfg: 'clang-{__clang_major__}'.format(**compilerMacros(cfg)),                                                when=_isClang),
   Feature(name=lambda cfg: 'clang-{__clang_major__}.{__clang_minor__}'.format(**compilerMacros(cfg)),                              when=_isClang),
   Feature(name=lambda cfg: 'clang-{__clang_major__}.{__clang_minor__}.{__clang_patchlevel__}'.format(**compilerMacros(cfg)),       when=_isClang),
 
   Feature(name='gcc',                                                                                                              when=_isGCC),
   Feature(name=lambda cfg: 'gcc-{__GNUC__}'.format(**compilerMacros(cfg)),                                                         when=_isGCC),
   Feature(name=lambda cfg: 'gcc-{__GNUC__}.{__GNUC_MINOR__}'.format(**compilerMacros(cfg)),                                        when=_isGCC),
   Feature(name=lambda cfg: 'gcc-{__GNUC__}.{__GNUC_MINOR__}.{__GNUC_PATCHLEVEL__}'.format(**compilerMacros(cfg)),                  when=_isGCC),
+
+  Feature(name='msvc',                                                                                                             when=_isMSVC),
+  Feature(name=lambda cfg: 'msvc-{}'.format(*_msvcVersion(cfg)),                                                                   when=_isMSVC),
+  Feature(name=lambda cfg: 'msvc-{}.{}'.format(*_msvcVersion(cfg)),                                                                when=_isMSVC),
 ]
 
 # Deduce and add the test features that that are implied by the #defines in
 # the <__config_site> header.
 #
 # For each macro of the form `_LIBCPP_XXX_YYY_ZZZ` defined below that
 # is defined after including <__config_site>, add a Lit feature called
 # `libcpp-xxx-yyy-zzz`. When a macro is defined to a specific value

libcxx/utils/libcxx/test/params.py

 #===----------------------------------------------------------------------===##
 #
 # Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 #===----------------------------------------------------------------------===##
 
 from libcxx.test.dsl import *
+from libcxx.test.features import _isMSVC
 
 _warningFlags = [
   '-Werror',
   '-Wall',
   '-Wextra',
   '-Wshadow',
   '-Wundef',
   '-Wno-unused-command-line-argument',
Parameter(name='debug_level', choices=['', '0', '1'], type=str, default='',
               AddCompileFlag('-D_LIBCPP_DEBUG={}'.format(debugLevel))
             ]),
 
   # Parameters to enable or disable parts of the test suite
   Parameter(name='enable_experimental', choices=[True, False], type=bool, default=False,
             help="Whether to enable tests for experimental C++ libraries (typically Library Fundamentals TSes).",
             actions=lambda experimental: [] if not experimental else [
               AddFeature('c++experimental'),
-              PrependLinkFlag('-lc++experimental')
+              # When linking in MSVC mode via the Clang driver, a -l<foo>
+              # maps to <foo>.lib, so we need to use -llibc++experimental here
+              # to make it link against the static libc++experimental.lib.
+              # We can't check for the feature 'msvc' in available_features
+              # as those features are added after processing parameters.
+              PrependLinkFlag(lambda config: '-llibc++experimental' if _isMSVC(config) else '-lc++experimental')
             ]),
 
   Parameter(name='long_tests', choices=[True, False], type=bool, default=True,
             help="Whether to enable tests that take longer to run. This can be useful when running on a very slow device.",
             actions=lambda enabled: [] if not enabled else [
               AddFeature('long_tests')
             ]),
 
   Parameter(name='enable_debug_tests', choices=[True, False], type=bool, default=True,
             help="Whether to enable tests that exercise the libc++ debugging mode.",
             actions=lambda enabled: [] if enabled else [
               AddFeature('libcxx-no-debug-mode')
             ]),
 ]