[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