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

[ldionne]

Summary

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.

[Mordante]

Thanks a lot. I just tested the patch locally on a branch where I had modular build issues. With this patch it again was easy to solve these issues!
LGTM.

[ldionne]

Fix the dsl tests.

[• Quuxplusone]

I'd just done print(err) on old line 210, in my local checkout, but I assume your raise is better. :)

[ldionne]

In D114010#3143964, @Quuxplusone wrote:

I'd just done print(err) on old line 210, in my local checkout, but I assume your raise is better. :)

Well, the benefit is that the exception will bubble up all the way and end Lit configuration, which is really what we want. And then its payload (the string) will be printed, which is also what we want.

[ldionne]

Rebase onto main now that the sanitizers are fixed.

[ldionne]

I will merge this once CI passes. I needed to land D114385 to fix the sanitizers build.

libcxx/test/libcxx/selftest/dsl/dsl.sh.py

def test_valid_program_returns_output_newline_handling(self):
         self.assertEqual(dsl.programOutput(self.config, source), "FOOBAR\n")
 
     def test_valid_program_returns_no_output(self):
         source = """
         int main(int, char**) { return 0; }
         """
         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 = """
         #include <cassert>
         #include <string>
         int main(int argc, char** argv) {
             assert(argc == 3);
             assert(argv[1] == std::string("first-argument"));
def test_doesnt_explode(self):
         try:
             dsl.hasAnyLocale(self.config, ['en_US.UTF-8'])
         except subprocess.CalledProcessError:
             self.fail("checking for hasLocale should not explode")
 
     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):
     """
     Tests for libcxx.test.dsl.compilerMacros
     """
     def test_basic(self):
         macros = dsl.compilerMacros(self.config)
         self.assertIsInstance(macros, dict)

libcxx/utils/libcxx/test/dsl.py

 
 import libcxx.test.format
 import lit
 import lit.LitConfig
 import lit.Test
 import lit.TestRunner
 import lit.util
 
+class ConfigurationError(Exception):
+  pass
+
+class ConfigurationCompilationError(ConfigurationError):
+  pass
+
+class ConfigurationRuntimeError(ConfigurationError):
+  pass
 
 def _memoizeExpensiveOperation(extractCacheKey):
   """
   Allows memoizing a very expensive operation.
 
   We pickle the cache key to make sure we store an immutable representation
   of it. If we stored an object and the object was referenced elsewhere, it
   could be changed from under our feet, which would break the cache.
def sourceBuilds(config, source):
   Return whether the program in the given string builds successfully.
 
   This is done by compiling and linking a program that consists of the given
   source with the %{cxx} substitution, and seeing whether that succeeds.
   """
   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))
 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 ""
     return actualOut
 
 @_memoizeExpensiveOperation(lambda c, f: (c.substitutions, c.environment, f))
 def hasCompileFlag(config, flag):
   """
def hasAnyLocale(config, locales):
   whether any of the passed locale names is supported by setlocale() and returns
   true if one of them works.
 
   This is done by executing a program that tries to set the given locale using
   %{exec} -- this means that the command may be executed on a remote host
   depending on the %{exec} substitution.
   """
   program = """
+    #include <stddef.h>
+    #if defined(_LIBCPP_HAS_NO_LOCALIZATION)
+      int main(int, char**) { return 1; }
+    #else
       #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;
           }
-        printf("%s is not supported.\\n", argv[i]);
         }
         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=''):
   """
   Return a dictionary of predefined compiler macros.
 
   The keys are strings representing macros, and the values are strings
   representing what each macro is defined to.
 
   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:
       line = line[len('#define '):]
       macro, _, value = line.partition(' ')
       parsedMacros[macro] = value
     return parsedMacros