[libcxx-commits] [libcxx] 008849d - [libcxx] [test] Don't rerun supportsVerify for each individual test

[Martin Storsjö via libcxx-commits]

Author: Martin Storsjö
Date: 2021-12-21T19:40:30+02:00
New Revision: 008849d7a51e4d7125d5b2eaeba495e4797dd749

URL: https://github.com/llvm/llvm-project/commit/008849d7a51e4d7125d5b2eaeba495e4797dd749
DIFF: https://github.com/llvm/llvm-project/commit/008849d7a51e4d7125d5b2eaeba495e4797dd749.diff

LOG: [libcxx] [test] Don't rerun supportsVerify for each individual test

We can't just memoize _supportsVerify in place in format.py, as it
previously was executed in each of the individual processes.

Instead use hasCompileFlag() and add a feature flag for it instead,
which can be used both by tests (that already have such a flag,
locally for one set of tests) and for the testing framework itself.

By using hasCompileFlag(), this also implicitly fixes two other issues:
Previously, _supportsVerify called subprocess.call() directly, which can
interpret command line quoting differently than lit.TestRunner.

(In particular, TestRunner handles arguments quoted by a single quote,
while launching Windows processes with subprocess.call() only supports
double quotes. This allows using shlex.quote(), which uses single quotes,
everywhere - as all commands now go through TestRunner. This should make
41d7909368bebc897467a75860a524a5f172564f redundant.)

Secondly, the old _supportsVerify method didn't include %{flags) or
%{compile_flags}.

Differential Revision: https://reviews.llvm.org/D116010

Added: 
    

Modified: 
    libcxx/utils/libcxx/test/features.py
    libcxx/utils/libcxx/test/format.py

Removed: 
    libcxx/test/libcxx/selftest/fail.cpp/lit.local.cfg


################################################################################
diff  --git a/libcxx/test/libcxx/selftest/fail.cpp/lit.local.cfg b/libcxx/test/libcxx/selftest/fail.cpp/lit.local.cfg
deleted file mode 100644
index b8ef6cb95c96c..0000000000000
--- a/libcxx/test/libcxx/selftest/fail.cpp/lit.local.cfg
+++ /dev/null
@@ -1,6 +0,0 @@
-import libcxx.test.format
-
-# The tests in this directory need to know whether Clang-verify is supported
-# to work properly.
-if libcxx.test.format._supportsVerify(config):
-    config.available_features.add('verify-support')

diff  --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 241d1b57a1a5e..e4755fec23167 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -41,6 +41,7 @@
   Feature(name='has-fobjc-arc',                 when=lambda cfg: hasCompileFlag(cfg, '-xobjective-c++ -fobjc-arc') and
                                                                  sys.platform.lower().strip() == 'darwin'), # TODO: this doesn't handle cross-compiling to Apple platforms.
   Feature(name='objective-c++',                 when=lambda cfg: hasCompileFlag(cfg, '-xobjective-c++ -fobjc-arc')),
+  Feature(name='verify-support',                when=lambda cfg: hasCompileFlag(cfg, '-Xclang -verify-ignore-unexpected')),
 
   Feature(name='non-lockfree-atomics',
           when=lambda cfg: sourceBuilds(cfg, """

diff  --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
index fdf26b67ac4e1..b53dfdc133346 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -12,21 +12,6 @@
 import pipes
 import re
 import shutil
-import subprocess
-
-def _supportsVerify(config):
-    """
-    Determine whether clang-verify is supported by the given configuration.
-
-    This is done by checking whether the %{cxx} substitution in that
-    configuration supports certain compiler flags.
-    """
-    command = "%{{cxx}} -xc++ {} -Werror -fsyntax-only -Xclang -verify-ignore-unexpected".format(os.devnull)
-    command = lit.TestRunner.applySubstitutions([command], config.substitutions,
-                                                recursion_limit=config.recursiveExpansionLimit)[0]
-    devNull = open(os.devnull, 'w')
-    result = subprocess.call(command, shell=True, stdout=devNull, stderr=devNull)
-    return result == 0
 
 def _getTempPaths(test):
     """
@@ -216,7 +201,7 @@ def _disableWithModules(self, test):
 
     def execute(self, test, litConfig):
         VERIFY_FLAGS = '-Xclang -verify -Xclang -verify-ignore-unexpected=note -ferror-limit=0'
-        supportsVerify = _supportsVerify(test.config)
+        supportsVerify = 'verify-support' in test.config.available_features
         filename = test.path_in_suite[-1]
 
         # TODO(ldionne): We currently disable tests that re-define _LIBCPP_ASSERT