[libcxx-commits] [libcxx] f18f9ce - [libc++] Properly handle errors happening during Lit configuration

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Fri Nov 26 08:03:23 PST 2021


Author: Louis Dionne
Date: 2021-11-26T11:03:15-05:00
New Revision: f18f9ce3665e51e5f09a3d32f9e36fd24e3a79f7

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

LOG: [libc++] Properly handle errors happening during Lit configuration

Instead of silently swallowing errors that happen during Lit configuration
(for example trying to obtain compiler macros but compiling fails), raise
an exception with some amount of helpful information.

This should avoid the possibility of silently configuring Lit in a bogus
way, and also provides more helpful information when things fail.

Note that this requires a bit more finesse around how we handle some
failing configuration checks that we would previously return None for.

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

Added: 
    

Modified: 
    libcxx/test/libcxx/selftest/dsl/dsl.sh.py
    libcxx/utils/libcxx/test/dsl.py

Removed: 
    


################################################################################
diff  --git a/libcxx/test/libcxx/selftest/dsl/dsl.sh.py b/libcxx/test/libcxx/selftest/dsl/dsl.sh.py
index 16435a073fcec..5da2cc9f045ba 100644
--- a/libcxx/test/libcxx/selftest/dsl/dsl.sh.py
+++ b/libcxx/test/libcxx/selftest/dsl/dsl.sh.py
@@ -151,19 +151,19 @@ def test_valid_program_returns_no_output(self):
         """
         self.assertEqual(dsl.programOutput(self.config, source), "")
 
-    def test_invalid_program_returns_None_1(self):
+    def test_program_that_fails_to_run_raises_runtime_error(self):
         # The program compiles, but exits with an error
         source = """
         int main(int, char**) { return 1; }
         """
-        self.assertEqual(dsl.programOutput(self.config, source), None)
+        self.assertRaises(dsl.ConfigurationRuntimeError, lambda: dsl.programOutput(self.config, source))
 
-    def test_invalid_program_returns_None_2(self):
+    def test_program_that_fails_to_compile_raises_compilation_error(self):
         # The program doesn't compile
         source = """
         int main(int, char**) { this doesnt compile }
         """
-        self.assertEqual(dsl.programOutput(self.config, source), None)
+        self.assertRaises(dsl.ConfigurationCompilationError, lambda: dsl.programOutput(self.config, source))
 
     def test_pass_arguments_to_program(self):
         source = """
@@ -231,6 +231,11 @@ def test_doesnt_explode(self):
     def test_nonexistent_locale(self):
         self.assertFalse(dsl.hasAnyLocale(self.config, ['for_sure_this_is_not_an_existing_locale']))
 
+    def test_localization_program_doesnt_compile(self):
+        compilerIndex = findIndex(self.config.substitutions, lambda x: x[0] == '%{cxx}')
+        self.config.substitutions[compilerIndex] = ('%{cxx}', 'this-is-certainly-not-a-valid-compiler!!')
+        self.assertRaises(dsl.ConfigurationCompilationError, lambda: dsl.hasAnyLocale(self.config, ['en_US.UTF-8']))
+
 
 class TestCompilerMacros(SetupConfigs):
     """

diff  --git a/libcxx/utils/libcxx/test/dsl.py b/libcxx/utils/libcxx/test/dsl.py
index a9d45f2b5a598..9042e7c21a0a4 100644
--- a/libcxx/utils/libcxx/test/dsl.py
+++ b/libcxx/utils/libcxx/test/dsl.py
@@ -21,6 +21,14 @@
 import lit.TestRunner
 import lit.util
 
+class ConfigurationError(Exception):
+  pass
+
+class ConfigurationCompilationError(ConfigurationError):
+  pass
+
+class ConfigurationRuntimeError(ConfigurationError):
+  pass
 
 def _memoizeExpensiveOperation(extractCacheKey):
   """
@@ -115,7 +123,7 @@ def sourceBuilds(config, source):
   with _makeConfigTest(config) as test:
     with open(test.getSourcePath(), 'w') as sourceFile:
       sourceFile.write(source)
-    out, err, exitCode, timeoutInfo = _executeScriptInternal(test, ['%{build}'])
+    _, _, exitCode, _ = _executeScriptInternal(test, ['%{build}'])
     return exitCode == 0
 
 @_memoizeExpensiveOperation(lambda c, p, args=None: (c.substitutions, c.environment, p, args))
@@ -124,22 +132,22 @@ def programOutput(config, program, args=None):
   Compiles a program for the test target, run it on the test target and return
   the output.
 
-  If the program fails to compile or run, None is returned instead. Note that
-  execution of the program is done through the %{exec} substitution, which means
-  that the program may be run on a remote host depending on what %{exec} does.
+  Note that execution of the program is done through the %{exec} substitution,
+  which means that the program may be run on a remote host depending on what
+  %{exec} does.
   """
   if args is None:
     args = []
   with _makeConfigTest(config) as test:
     with open(test.getSourcePath(), 'w') as source:
       source.write(program)
-    _, _, exitCode, _ = _executeScriptInternal(test, ['%{build}'])
+    _, err, exitCode, _ = _executeScriptInternal(test, ['%{build}'])
     if exitCode != 0:
-      return None
+      raise ConfigurationCompilationError("Failed to build program, stderr is:\n{}".format(err))
 
     out, err, exitCode, _ = _executeScriptInternal(test, ["%{{run}} {}".format(' '.join(args))])
     if exitCode != 0:
-      return None
+      raise ConfigurationRuntimeError("Failed to run program, stderr is:\n{}".format(err))
 
     actualOut = re.search("# command output:\n(.+)\n$", out, flags=re.DOTALL)
     actualOut = actualOut.group(1) if actualOut else ""
@@ -172,21 +180,26 @@ def hasAnyLocale(config, locales):
   depending on the %{exec} substitution.
   """
   program = """
-    #include <locale.h>
-    #include <stdio.h>
-    int main(int argc, char** argv) {
-      // For debugging purposes print which locales are (not) supported.
-      for (int i = 1; i < argc; i++) {
-        if (::setlocale(LC_ALL, argv[i]) != NULL) {
-          printf("%s is supported.\\n", argv[i]);
-          return 0;
+    #include <stddef.h>
+    #if defined(_LIBCPP_HAS_NO_LOCALIZATION)
+      int main(int, char**) { return 1; }
+    #else
+      #include <locale.h>
+      int main(int argc, char** argv) {
+        for (int i = 1; i < argc; i++) {
+          if (::setlocale(LC_ALL, argv[i]) != NULL) {
+            return 0;
+          }
         }
-        printf("%s is not supported.\\n", argv[i]);
+        return 1;
       }
-      return 1;
-    }
+    #endif
   """
-  return programOutput(config, program, args=[pipes.quote(l) for l in locales]) is not None
+  try:
+    programOutput(config, program, args=[pipes.quote(l) for l in locales])
+  except ConfigurationRuntimeError:
+    return False
+  return True
 
 @_memoizeExpensiveOperation(lambda c, flags='': (c.substitutions, c.environment, flags))
 def compilerMacros(config, flags=''):
@@ -198,20 +211,17 @@ def compilerMacros(config, flags=''):
 
   If the optional `flags` argument (a string) is provided, these flags will
   be added to the compiler invocation when generating the macros.
-
-  If we fail to extract the compiler macros because of a compiler error, None
-  is returned instead.
   """
   with _makeConfigTest(config) as test:
     with open(test.getSourcePath(), 'w') as sourceFile:
       # Make sure files like <__config> are included, since they can define
       # additional macros.
       sourceFile.write("#include <stddef.h>")
-    unparsedOutput, err, exitCode, timeoutInfo = _executeScriptInternal(test, [
+    unparsedOutput, err, exitCode, _ = _executeScriptInternal(test, [
       "%{{cxx}} %s -dM -E %{{flags}} %{{compile_flags}} {}".format(flags)
     ])
     if exitCode != 0:
-      return None
+      raise ConfigurationCompilationError("Failed to retrieve compiler macros, stderr is:\n{}".format(err))
     parsedMacros = dict()
     defines = (l.strip() for l in unparsedOutput.split('\n') if l.startswith('#define '))
     for line in defines:


        


More information about the libcxx-commits mailing list