[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