[libcxx-commits] [libcxx] 9020d28 - [libcxx][lit] Fix incorrect lambda capture in hasLocale checks

[Alex Richardson via libcxx-commits]

Author: Alex Richardson
Date: 2020-07-23T11:19:18+01:00
New Revision: 9020d28688492c437abb648b6ab69baeba523219

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

LOG: [libcxx][lit] Fix incorrect lambda capture in hasLocale checks

The lambda being used to check whether locales are supported was always
passing the value of alts from the last loop iteration due to the way that
python lambda captures work. Fix this by using a default argument capture.

To help debug future similar issues I also added a prefix to the config
test binary indicating which locale is being tested.
I originally found this issue when implementing a new executor that simply
collects test binaries in a given directory and was surprised to see many
additional executables other than the expected test binaries. I therefore
added the locale prefix to the test binaries and noticed that they were all
checking for cs_CZ.ISO8859-2.

Reviewed By: #libc, ldionne

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/libcxx/utils/libcxx/test/dsl.py b/libcxx/utils/libcxx/test/dsl.py
index 2c54921844b2..cd500e132946 100644
--- a/libcxx/utils/libcxx/test/dsl.py
+++ b/libcxx/utils/libcxx/test/dsl.py
@@ -52,13 +52,14 @@ def _executeScriptInternal(test, commands):
     res = ('', '', 127, None)
   return res
 
-def _makeConfigTest(config):
+def _makeConfigTest(config, testPrefix=None):
   sourceRoot = os.path.join(config.test_exec_root, '__config_src__')
   execRoot = os.path.join(config.test_exec_root, '__config_exec__')
   suite = lit.Test.TestSuite('__config__', sourceRoot, execRoot, config)
   if not os.path.exists(sourceRoot):
     os.makedirs(sourceRoot)
-  tmp = tempfile.NamedTemporaryFile(dir=sourceRoot, delete=False, suffix='.cpp')
+  tmp = tempfile.NamedTemporaryFile(dir=sourceRoot, delete=False, suffix='.cpp',
+                                    prefix=testPrefix)
   tmp.close()
   pathInSuite = [os.path.relpath(tmp.name, sourceRoot)]
   class TestWrapper(lit.Test.Test):
@@ -82,7 +83,7 @@ def sourceBuilds(config, source):
     _executeScriptInternal(test, ['rm %t.exe'])
     return exitCode == 0
 
-def programOutput(config, program, args=[]):
+def programOutput(config, program, args=[], testPrefix=None):
   """
   Compiles a program for the test target, run it on the test target and return
   the output.
@@ -91,7 +92,7 @@ def programOutput(config, program, args=[]):
   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.
   """
-  with _makeConfigTest(config) as test:
+  with _makeConfigTest(config, testPrefix=testPrefix) as test:
     with open(test.getSourcePath(), 'w') as source:
       source.write(program)
     try:
@@ -142,7 +143,8 @@ def hasLocale(config, locale):
       else                                      return 1;
     }
   """
-  return programOutput(config, program, args=[pipes.quote(locale)]) != None
+  return programOutput(config, program, args=[pipes.quote(locale)],
+                       testPrefix="check_locale_" + locale) is not None
 
 def compilerMacros(config, flags=''):
   """

diff  --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 6a16ca851d3f..dfdc0bae7a29 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -104,10 +104,10 @@
   'cs_CZ.ISO8859-2': ['cs_CZ.ISO8859-2', 'Czech_Czech Republic.1250']
 }
 for locale, alts in locales.items():
-  DEFAULT_FEATURES += [
-    Feature(name='locale.{}'.format(locale),
-            when=lambda cfg: any(hasLocale(cfg, alt) for alt in alts))
-  ]
+  # Note: Using alts directly in the lambda body here will bind it to the value at the
+  # end of the loop. Assigning it to a default argument works around this issue.
+  DEFAULT_FEATURES.append(Feature(name='locale.{}'.format(locale),
+                                  when=lambda cfg, alts=alts: any(hasLocale(cfg, alt) for alt in alts)))
 
 
 # Add features representing the platform name: darwin, linux, windows, etc...