[PATCH] D80786: Rename APIs in unittests/AST/Language.h in preparation to share them
Gabor Marton via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri May 29 04:50:58 PDT 2020
martong added a comment.
Thank you for working on this.
> moving into a clang::test namespace instead of prefixing the names
+1 for this. We could go even further: `clang::test::ast` or `clang::unittest::ast`.
================
Comment at: clang/unittests/AST/Language.h:23
+
+enum UnittestLanguage {
+ Lang_C,
----------------
sammccall wrote:
> 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();
> }
> ```
Yes I agree. However, in the ASTImporter tests we didn't need to select multiple languages for one test case (yet). I think later if we need that case then we can address this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80786/new/
https://reviews.llvm.org/D80786
More information about the cfe-commits
mailing list