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

[Alexander Richardson via Phabricator via libcxx-commits]

arichardson updated this revision to Diff 278841.
arichardson marked 3 inline comments as done.
arichardson added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84040/new/

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
@@ -104,10 +104,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: 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.
+  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...
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, 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="dsl_config_" if testPrefix is None else testPrefix)
   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=[], testPrefix=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, testPrefix=testPrefix) as test:
     with open(test.getSourcePath(), 'w') as source:
       source.write(program)
     try:
@@ -142,7 +143,8 @@
       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=''):
   """


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