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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Nice catch! This is definitely the corner of Python that causes me the most trouble, I keep getting bitten by it.



================
Comment at: libcxx/utils/libcxx/test/dsl.py:55
 
-def _makeConfigTest(config):
+def _makeConfigTest(config, test_name=None):
   sourceRoot = os.path.join(config.test_exec_root, '__config_src__')
----------------
I would use something like `testSlug` (in reference to git repo slugs, etc) instead of `test_name`. It's not really the name of the test, it's just part of it.


================
Comment at: libcxx/utils/libcxx/test/dsl.py:146
   """
-  return programOutput(config, program, args=[pipes.quote(locale)]) != None
+  for locale in locales:
+    if programOutput(config, program, args=[pipes.quote(locale)],
----------------
I'd rather keep this checking for a single locale.


================
Comment at: libcxx/utils/libcxx/test/features.py:110
+  # end of the loop. Use functools.partial instead:
+  features.append(Feature(name='locale.{}'.format(locale),
+                          when=functools.partial(hasLocale, locales=alts)))
----------------
I think you can also use:

```
lambda cfg, alts=alts: any(hasLocale(cfg, alt) for alt in alts)
```

I think it's more idiomatic in Python, and it seems easier to understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84040