Skip to content

Commit 9020d28

Browse files
committed
[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
1 parent 1162ffe commit 9020d28

File tree

2 files changed

+11
-9
lines changed

2 files changed

+11
-9
lines changed

libcxx/utils/libcxx/test/dsl.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,14 @@ def _executeScriptInternal(test, commands):
5252
res = ('', '', 127, None)
5353
return res
5454

55-
def _makeConfigTest(config):
55+
def _makeConfigTest(config, testPrefix=None):
5656
sourceRoot = os.path.join(config.test_exec_root, '__config_src__')
5757
execRoot = os.path.join(config.test_exec_root, '__config_exec__')
5858
suite = lit.Test.TestSuite('__config__', sourceRoot, execRoot, config)
5959
if not os.path.exists(sourceRoot):
6060
os.makedirs(sourceRoot)
61-
tmp = tempfile.NamedTemporaryFile(dir=sourceRoot, delete=False, suffix='.cpp')
61+
tmp = tempfile.NamedTemporaryFile(dir=sourceRoot, delete=False, suffix='.cpp',
62+
prefix=testPrefix)
6263
tmp.close()
6364
pathInSuite = [os.path.relpath(tmp.name, sourceRoot)]
6465
class TestWrapper(lit.Test.Test):
@@ -82,7 +83,7 @@ def sourceBuilds(config, source):
8283
_executeScriptInternal(test, ['rm %t.exe'])
8384
return exitCode == 0
8485

85-
def programOutput(config, program, args=[]):
86+
def programOutput(config, program, args=[], testPrefix=None):
8687
"""
8788
Compiles a program for the test target, run it on the test target and return
8889
the output.
@@ -91,7 +92,7 @@ def programOutput(config, program, args=[]):
9192
execution of the program is done through the %{exec} substitution, which means
9293
that the program may be run on a remote host depending on what %{exec} does.
9394
"""
94-
with _makeConfigTest(config) as test:
95+
with _makeConfigTest(config, testPrefix=testPrefix) as test:
9596
with open(test.getSourcePath(), 'w') as source:
9697
source.write(program)
9798
try:
@@ -142,7 +143,8 @@ def hasLocale(config, locale):
142143
else return 1;
143144
}
144145
"""
145-
return programOutput(config, program, args=[pipes.quote(locale)]) != None
146+
return programOutput(config, program, args=[pipes.quote(locale)],
147+
testPrefix="check_locale_" + locale) is not None
146148

147149
def compilerMacros(config, flags=''):
148150
"""

libcxx/utils/libcxx/test/features.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,10 @@
104104
'cs_CZ.ISO8859-2': ['cs_CZ.ISO8859-2', 'Czech_Czech Republic.1250']
105105
}
106106
for locale, alts in locales.items():
107-
DEFAULT_FEATURES += [
108-
Feature(name='locale.{}'.format(locale),
109-
when=lambda cfg: any(hasLocale(cfg, alt) for alt in alts))
110-
]
107+
# Note: Using alts directly in the lambda body here will bind it to the value at the
108+
# end of the loop. Assigning it to a default argument works around this issue.
109+
DEFAULT_FEATURES.append(Feature(name='locale.{}'.format(locale),
110+
when=lambda cfg, alts=alts: any(hasLocale(cfg, alt) for alt in alts)))
111111

112112

113113
# Add features representing the platform name: darwin, linux, windows, etc...

0 commit comments

Comments
 (0)