[PATCH] D80786: Rename APIs in unittests/AST/Language.h in preparation to share them

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

In D80786#2062557 <https://reviews.llvm.org/D80786#2062557>, @sammccall wrote:

> - Unittest -> Test (shorter, and unit test is two words, and llvm naming conventions notwithstanding many gtests are not unit tests)


Done.

> - UnitTestClangArgs --> `std::vector<std::string>` it's harder to parse but much more widely understood

Done. I also wanted to expand the typedef, but didn't do it because I didn't want to make this change controversial.

> - moving into a clang::test namespace instead of prefixing the names

Nested namespaces with names that can appear in other namespaces ("test") are a trap in C++: https://abseil.io/tips/130. This might not be a problem within the llvm-project.git code itself, but Clang is used as a library in a lot of other software.



================
Comment at: clang/unittests/AST/Language.h:23
+
+enum UnittestLanguage {
+  Lang_C,
----------------
martong wrote:
> 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.
Oh I absolutely agree -- I think we need a way to describe a lot more than just the language standard. Feature flags (like Objective-C, delayed template parsing etc.) should compose with each other.

I'm thinking it should be somewhat similar to the LangOptions struct, but be much friendlier for unit tests, the testing library should provide a bunch of presets that imitate important platforms (for example, 32-bit Linux, 64-bit Linux, macOS, iOS, Windows with delayed template parsing etc.)

However, we have to take this one step at a time.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80786/new/

https://reviews.llvm.org/D80786