[libcxx-commits] [PATCH] D84040: [libcxx][lit] Fix incorrect lambda capture in hasLocale checks

[Alexander Richardson via Phabricator via libcxx-commits]

arichardson created this revision.
arichardson added reviewers: libc++, ldionne.
Herald added subscribers: libcxx-commits, dexonsmith.
Herald added a project: libc++.
Herald added 1 blocking reviewer(s): libc++.

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 functools.partial instead.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84040

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


Index: libcxx/utils/libcxx/test/features.py
===================================================================
--- libcxx/utils/libcxx/test/features.py
+++ libcxx/utils/libcxx/test/features.py
@@ -7,6 +7,7 @@
 #===----------------------------------------------------------------------===##
 
 from libcxx.test.dsl import *
+import functools
 import sys
 
 _isClang      = lambda cfg: '__clang__' in compilerMacros(cfg) and '__apple_build_version__' not in compilerMacros(cfg)
@@ -104,10 +105,10 @@
   'cs_CZ.ISO8859-2': ['cs_CZ.ISO8859-2', 'Czech_Czech Republic.1250']
 }
 for locale, alts in locales.items():
-  features += [
-    Feature(name='locale.{}'.format(locale),
-            when=lambda cfg: any(hasLocale(cfg, alt) for alt in alts))
-  ]
+  # Note: we can't use a plain lambda here since alts will bind to the value at the
+  # end of the loop. Use functools.partial instead:
+  features.append(Feature(name='locale.{}'.format(locale),
+                          when=functools.partial(hasLocale, locales=alts)))
 
 
 # Add features representing the platform name: darwin, linux, windows, etc...
Index: libcxx/utils/libcxx/test/dsl.py
===================================================================
--- libcxx/utils/libcxx/test/dsl.py
+++ libcxx/utils/libcxx/test/dsl.py
@@ -52,13 +52,14 @@
     res = ('', '', 127, None)
   return res
 
-def _makeConfigTest(config):
+def _makeConfigTest(config, test_name=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="dsl_config_" if not test_name else test_name)
   tmp.close()
   pathInSuite = [os.path.relpath(tmp.name, sourceRoot)]
   class TestWrapper(lit.Test.Test):
@@ -82,7 +83,7 @@
     _executeScriptInternal(test, ['rm %t.exe'])
     return exitCode == 0
 
-def programOutput(config, program, args=[]):
+def programOutput(config, program, args=[], test_name=None):
   """
   Compiles a program for the test target, run it on the test target and return
   the output.
@@ -91,7 +92,7 @@
   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, test_name=test_name) as test:
     with open(test.getSourcePath(), 'w') as source:
       source.write(program)
     try:
@@ -127,7 +128,7 @@
     ])
     return exitCode == 0
 
-def hasLocale(config, locale):
+def hasLocale(config, locales):
   """
   Return whether the runtime execution environment supports a given locale.
 
@@ -142,7 +143,11 @@
       else                                      return 1;
     }
   """
-  return programOutput(config, program, args=[pipes.quote(locale)]) != None
+  for locale in locales:
+    if programOutput(config, program, args=[pipes.quote(locale)],
+                     test_name="check_locale_" + locale) is not None:
+      return True
+  return False
 
 def compilerMacros(config, flags=''):
   """


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84040.278808.patch
Type: text/x-patch
Size: 3387 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20200717/d3ed780c/attachment.bin>