sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Agree with the rationale about moving the names out of `clang::ast_matchers`,
and the names not being safe to put into `clang::` as-is.
The names are a bit unwieldy though, and I wonder if all of them are patterns
we want to encourage using widely (at least if so, the names become more
important).
I'd consider:
- Unittest -> Test (shorter, and unit test is two words, and llvm naming
conventions notwithstanding many gtests are not unit tests)
- moving into a clang::test namespace instead of prefixing the names
- UnitTestClangArgs --> `std::vector<std::string>` it's harder to parse but
much more widely understood
Anyway, don't want to block this, this is an important step to sharing which is
worthwhile whether the names are my favourite or not :-)
================
Comment at: clang/unittests/AST/Language.h:23
+
+enum UnittestLanguage {
+ Lang_C,
----------------
this enum doesn't seem great: why is the default CXX (presumably) 98? How do
you get objc++ with 14 features?
(No need to address this now, but it's not clear this is a great API to spread
more widely. Vs e.g.
```
struct TestLanguage {
static TestLanguage CXX; // some arbitrary version
static TestLanguage C;
enum {
CXX98,
CXX11,
} CXXVersion;
vector<string> getCommandLineArgs();
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80786/new/
https://reviews.llvm.org/D80786
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
