[libcxx-commits] [libcxx] 68587af - [libc++] Move handling of convenience substitutions outside of config.py

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Thu Apr 16 14:30:21 PDT 2020


Author: Louis Dionne
Date: 2020-04-16T17:30:09-04:00
New Revision: 68587af9ad10dce24c5164db2f627d2552629f27

URL: https://github.com/llvm/llvm-project/commit/68587af9ad10dce24c5164db2f627d2552629f27
DIFF: https://github.com/llvm/llvm-project/commit/68587af9ad10dce24c5164db2f627d2552629f27.diff

LOG: [libc++] Move handling of convenience substitutions outside of config.py

These substitutions are strongly tied to the operation of the test
format, so it makes sense to have them defined by the test format
instead of the Lit configuration. They should be defined regardless
of which configuration is in use.

Added: 
    libcxx/test/libcxx/selftest/newformat/convenience_substitutions/build_run.sh.cpp
    libcxx/test/libcxx/selftest/newformat/convenience_substitutions/verify.sh.cpp

Modified: 
    libcxx/utils/libcxx/test/config.py
    libcxx/utils/libcxx/test/format.py
    libcxx/utils/libcxx/test/newformat.py

Removed: 
    


################################################################################
diff  --git a/libcxx/test/libcxx/selftest/newformat/convenience_substitutions/build_run.sh.cpp b/libcxx/test/libcxx/selftest/newformat/convenience_substitutions/build_run.sh.cpp
new file mode 100644
index 000000000000..e050f88cf009
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/newformat/convenience_substitutions/build_run.sh.cpp
@@ -0,0 +1,24 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// Make sure that we provide the %{build} and %{run} convenience substitutions.
+
+// FILE_DEPENDENCIES: %t.exe
+// RUN: %{build}
+// RUN: %{run} "HELLO"
+
+#include <cassert>
+#include <string>
+
+int main(int argc, char** argv) {
+  assert(argc == 2);
+
+  std::string arg = argv[1];
+  assert(arg == "HELLO");
+  return 0;
+}

diff  --git a/libcxx/test/libcxx/selftest/newformat/convenience_substitutions/verify.sh.cpp b/libcxx/test/libcxx/selftest/newformat/convenience_substitutions/verify.sh.cpp
new file mode 100644
index 000000000000..664d25826a8d
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/newformat/convenience_substitutions/verify.sh.cpp
@@ -0,0 +1,18 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// Make sure that we provide the %{verify} substitution. We can only test
+// this when the verify-support feature is enabled, and it's 
diff icult to
+// check that it's enabled when it should be, so we just trust that it is.
+
+// REQUIRES: verify-support
+// RUN: test -n "%{verify}"
+
+// RUN: %{cxx} %s %{flags} %{compile_flags} -fsyntax-only %{verify}
+
+// expected-no-diagnostics

diff  --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index b9085a4cb5bb..81cc976565a0 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -332,8 +332,6 @@ def configure_use_clang_verify(self):
             self.use_clang_verify = self.cxx.isVerifySupported()
             self.lit_config.note(
                 "inferred use_clang_verify as: %r" % self.use_clang_verify)
-        if self.use_clang_verify:
-                self.config.available_features.add('verify-support')
 
     def configure_use_thread_safety(self):
         '''If set, run clang with -verify on failing tests.'''
@@ -988,9 +986,6 @@ def configure_substitutions(self):
         sub.append(('%{compile_flags}', ' '.join(map(pipes.quote, self.cxx.compile_flags))))
         sub.append(('%{link_flags}',    ' '.join(map(pipes.quote, self.cxx.link_flags))))
         sub.append(('%{link_libcxxabi}', pipes.quote(self.cxx.link_libcxxabi_flag)))
-        if self.cxx.isVerifySupported():
-            sub.append(('%{verify}', ' '.join(self.cxx.verify_flags)))
-        sub.append(('%{build}',   '%{cxx} -o %t.exe %s %{flags} %{compile_flags} %{link_flags}'))
 
         # Configure exec prefix substitutions.
         # Configure run env substitution.
@@ -1010,7 +1005,6 @@ def configure_substitutions(self):
         sub.append(('%{exec}', '{} {} {} -- '.format(pipes.quote(sys.executable),
                                                      pipes.quote(executor),
                                                      ' '.join(exec_args))))
-        sub.append(('%{run}', '%{exec} %t.exe'))
         if self.get_lit_conf('libcxx_gdb'):
             sub.append(('%{libcxx_gdb}', self.get_lit_conf('libcxx_gdb')))
 

diff  --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
index 7ad7d0a51bd0..99f10d7f2105 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -134,6 +134,12 @@ def _execute(self, test, lit_config):
         data_files = [f if os.path.isabs(f) else os.path.join(local_cwd, f) for f in data_files]
         substitutions.append(('%{file_dependencies}', ' '.join(data_files)))
 
+        # Add other convenience substitutions
+        if self.cxx.isVerifySupported():
+            substitutions.append(('%{verify}', ' '.join(self.cxx.verify_flags)))
+        substitutions.append(('%{build}', '%{cxx} -o %t.exe %s %{flags} %{compile_flags} %{link_flags}'))
+        substitutions.append(('%{run}', '%{exec} %t.exe'))
+
         script = lit.TestRunner.applySubstitutions(script, substitutions,
                                                    recursion_limit=test.config.recursiveExpansionLimit)
 

diff  --git a/libcxx/utils/libcxx/test/newformat.py b/libcxx/utils/libcxx/test/newformat.py
index 5e8e90706eaf..f704b968b6da 100644
--- a/libcxx/utils/libcxx/test/newformat.py
+++ b/libcxx/utils/libcxx/test/newformat.py
@@ -10,6 +10,7 @@
 import os
 import pipes
 import re
+import subprocess
 
 class CxxStandardLibraryTest(lit.formats.TestFormat):
     """
@@ -39,6 +40,9 @@ class CxxStandardLibraryTest(lit.formats.TestFormat):
                               test otherwise. This is supported only for backwards
                               compatibility with the test suite.
 
+
+    Substitution requirements
+    ===============================
     The test format operates by assuming that each test's configuration provides
     the following substitutions, which it will reuse in the shell scripts it
     constructs:
@@ -49,11 +53,13 @@ class CxxStandardLibraryTest(lit.formats.TestFormat):
         %{exec}          - A command to prefix the execution of executables
 
     Note that when building an executable (as opposed to only compiling a source
-    file), all three of ${flags}, %{compile_flags} and %{link_flags} will be used
+    file), all three of %{flags}, %{compile_flags} and %{link_flags} will be used
     in the same command line. In other words, the test format doesn't perform
     separate compilation and linking steps in this case.
 
 
+    Additional supported directives
+    ===============================
     In addition to everything that's supported in Lit ShTests, this test format
     also understands the following directives inside test files:
 
@@ -76,7 +82,39 @@ class CxxStandardLibraryTest(lit.formats.TestFormat):
             .sh.cpp test, which would be more powerful but perhaps overkill.
 
 
-    Design note:
+    Additional provided substitutions and features
+    ==============================================
+    The test format will define the following substitutions for use inside
+    tests:
+
+        %{verify}
+
+            This expands to the set of flags that must be passed to the
+            compiler in order to use Clang-verify, if that is supported.
+
+        verify-support
+
+            This Lit feature will be made available when the compiler supports
+            Clang-verify. This can be used to disable tests that require that
+            feature, such as `.verify.cpp` tests.
+
+        %{file_dependencies}
+
+            Expands to the list of files that this test depends on.
+            See FILE_DEPENDENCIES above.
+
+        %{build}
+            Expands to a command-line that builds the current source
+            file with the %{flags}, %{compile_flags} and %{link_flags}
+            substitutions, and that produces an executable named %t.exe.
+
+        %{run}
+            Equivalent to `%{exec} %t.exe`. This is intended to be used
+            in conjunction with the %{build} substitution.
+
+
+    Design notes
+    ============
     This test format never implicitly disables a type of test. For example,
     we could be tempted to automatically mark `.verify.cpp` tests as
     UNSUPPORTED when clang-verify isn't supported by the compiler. However,
@@ -104,7 +142,7 @@ def getTestsInDirectory(self, testSuite, pathInSuite, litConfig, localConfig):
                 if any([re.search(ext, filename) for ext in SUPPORTED_SUFFIXES]):
                     yield lit.Test.Test(testSuite, pathInSuite + (filename,), localConfig)
 
-    def _checkSubstitutions(self, substitutions):
+    def _checkBaseSubstitutions(self, substitutions):
         substitutions = [s for (s, _) in substitutions]
         for s in ['%{cxx}', '%{compile_flags}', '%{link_flags}', '%{flags}', '%{exec}']:
             assert s in substitutions, "Required substitution {} was not provided".format(s)
@@ -112,11 +150,11 @@ def _checkSubstitutions(self, substitutions):
     # Determine whether clang-verify is supported.
     def _supportsVerify(self, test, litConfig):
         command = "echo | %{cxx} -xc++ - -Werror -fsyntax-only -Xclang -verify-ignore-unexpected"
-        result = lit.TestRunner.executeShTest(test, litConfig,
-                                              useExternalSh=True,
-                                              preamble_commands=[command])
-        compilerSupportsVerify = result.code != lit.Test.FAIL
-        return compilerSupportsVerify
+        command = lit.TestRunner.applySubstitutions([command], test.config.substitutions,
+                                                    recursion_limit=test.config.recursiveExpansionLimit)[0]
+        devNull = open(os.devnull, 'w')
+        result = subprocess.call(command, shell=True, stdout=devNull, stderr=devNull)
+        return result == 0
 
     def _disableWithModules(self, test, litConfig):
         with open(test.getSourcePath(), 'rb') as f:
@@ -124,8 +162,7 @@ def _disableWithModules(self, test, litConfig):
         return b'#define _LIBCPP_ASSERT' in contents
 
     def execute(self, test, litConfig):
-        self._checkSubstitutions(test.config.substitutions)
-        VERIFY_FLAGS = '-Xclang -verify -Xclang -verify-ignore-unexpected=note -ferror-limit=0'
+        self._checkBaseSubstitutions(test.config.substitutions)
         filename = test.path_in_suite[-1]
 
         # TODO(ldionne): We currently disable tests that re-define _LIBCPP_ASSERT
@@ -168,7 +205,7 @@ def execute(self, test, litConfig):
             return self._executeShTest(test, litConfig, steps, fileDependencies=['%t.exe'])
         elif filename.endswith('.verify.cpp'):
             steps = [
-                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} -fsyntax-only " + VERIFY_FLAGS
+                "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} -fsyntax-only %{verify}"
             ]
             return self._executeShTest(test, litConfig, steps)
         # Make sure to check these ones last, since they will match other
@@ -185,7 +222,7 @@ def execute(self, test, litConfig):
         elif filename.endswith('.fail.cpp'):
             if self._supportsVerify(test, litConfig):
                 steps = [
-                    "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} -fsyntax-only " + VERIFY_FLAGS
+                    "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} -fsyntax-only %{verify}"
                 ]
             else:
                 steps = [
@@ -205,6 +242,22 @@ def _executeShTest(self, test, litConfig, steps, fileDependencies=None):
         if test.config.unsupported:
             return lit.Test.Result(lit.Test.UNSUPPORTED, 'Test is unsupported')
 
+        # Get the default substitutions
+        tmpDir, tmpBase = lit.TestRunner.getTempPaths(test)
+        useExternalSh = True
+        substitutions = lit.TestRunner.getDefaultSubstitutions(test, tmpDir, tmpBase,
+                                                               normalize_slashes=useExternalSh)
+
+        # Add the %{build} and %{run} convenience substitutions
+        substitutions.append(('%{build}', '%{cxx} %s %{flags} %{compile_flags} %{link_flags} -o %t.exe'))
+        substitutions.append(('%{run}', '%{exec} %t.exe'))
+
+        # Add the %{verify} substitution and the verify-support feature if Clang-verify is supported
+        if self._supportsVerify(test, litConfig):
+            test.config.available_features.add('verify-support')
+            substitutions.append(('%{verify}', '-Xclang -verify -Xclang -verify-ignore-unexpected=note -ferror-limit=0'))
+
+        # Parse the test file, including custom directives
         additionalCompileFlags = []
         fileDependencies = fileDependencies or []
         parsers = [
@@ -223,16 +276,9 @@ def _executeShTest(self, test, litConfig, steps, fileDependencies=None):
             return parsed
         script += parsed
 
-        if litConfig.noExecute:
-            return lit.Test.Result(lit.Test.PASS)
-
         # Add compile flags specified with ADDITIONAL_COMPILE_FLAGS.
-        self.addCompileFlags(test.config, *additionalCompileFlags)
-
-        tmpDir, tmpBase = lit.TestRunner.getTempPaths(test)
-        useExternalSh = True
-        substitutions = lit.TestRunner.getDefaultSubstitutions(test, tmpDir, tmpBase,
-                                                               normalize_slashes=useExternalSh)
+        substitutions = [(s, x + ' ' + ' '.join(additionalCompileFlags)) if s == '%{compile_flags}'
+                                else (s, x) for (s, x) in substitutions]
 
         # Perform substitutions inside FILE_DEPENDENCIES lines (or injected dependencies).
         # This allows using variables like %t in file dependencies. Also note that we really
@@ -251,4 +297,7 @@ def _executeShTest(self, test, litConfig, steps, fileDependencies=None):
         script = lit.TestRunner.applySubstitutions(script, substitutions,
                                                    recursion_limit=test.config.recursiveExpansionLimit)
 
-        return lit.TestRunner._runShTest(test, litConfig, useExternalSh, script, tmpBase)
+        if litConfig.noExecute:
+            return lit.Test.Result(lit.Test.PASS)
+        else:
+            return lit.TestRunner._runShTest(test, litConfig, useExternalSh, script, tmpBase)


        


More information about the libcxx-commits mailing list