[PATCH] D68064: [Builtins] Provide a mechanism to selectively disable tests based on whether an implementation is provided by a builtin library.

Petr Hosek via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 1 23:41:05 PDT 2019


phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM % a few nits



================
Comment at: test/builtins/Unit/lit.cfg.py:101
+# Sanity checks
+if len(builtins_source_features) == 0:
+  lit_config.fatal('builtins_source_features cannot be empty')
----------------
No need to check the length, `if builtins_source_features:` is sufficient.


================
Comment at: test/builtins/Unit/lit.cfg.py:108
+    lit_config.fatal('builtins_source_features cannot contain empty features')
+  old_size = len(builtins_source_features_set)
+  builtins_source_features_set.add(builtin_source_feature)
----------------
I'd slightly prefer to do:
```
if builtin_source_feature not in builtins_source_features_set:
  builtins_source_features_set.add(builtin_source_feature)
else:
  builtins_source_feature_duplicates.append(builtin_source_feature)
```
It seems easier to comprehend.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D68064





More information about the llvm-commits mailing list