[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