[PATCH] D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies

[Gabor Marton via Phabricator via cfe-commits]

martong marked 3 inline comments as done.
martong added inline comments.


================
Comment at: clang/unittests/AST/ASTImporterODRStrategiesTest.cpp:118
+      template <class T>
+      constexpr T X;
+      )";
----------------
shafik wrote:
> shafik wrote:
> > Note this is not well-formed b/c it is not initialized, [see godbolt](https://godbolt.org/z/8xvFwh)
> But it would be ok combined w/ a specialization:
> 
> ```
> template <>
> constexpr int X<int> = 0;
> ```
Ok, I changed the template to have an init expression:
```
template <class T>
constexpr T X = 0;
```


================
Comment at: clang/unittests/AST/ASTImporterODRStrategiesTest.cpp:151
+};
+
+template <typename TypeParam, ASTImporter::ODRHandlingType ODRHandlingParam>
----------------
shafik wrote:
> martong wrote:
> > balazske wrote:
> > > `FunctionTemplate` and `FunctionTemplateSpec` are missing?
> > Yes, because `FunctionTemplates` overload with each other. So they are imported always "liberally".
> > 
> > There is no point to liberally import conflicting `FunctionTemplateSpecializations`.
> > The only thing we can do in that case is to omit the conflicting declaration.
> > And this is true in case of `ClassTemplateSpecialization`s too.
> > 
> > Perhaps we should remove `struct ClassTemplateSpec` as well from here (?).
> > Because they are never going to be handled "liberally".
> > 
> > @shafik , what do you think about this?
> What you say about `FunctionTemplateSpecializations` and `ClassTemplateSpecializations` seems to make sense, importing them liberally would require more than work in the importer.
I have added `FunctionTemplate`, `FunctionTemplateSpec` and `VarTemplateSpec`.

FunctionTemplate decls overload with each other. Thus, they are imported always "liberally". I have added a test for that.

Class and variable template specializations/instantiatons are always imported conservatively, because the AST holds the specializations in a set, and the key within the set is a hash calculated from the arguments of the specialization.
I have added tests for class template specializations, but put the VarTemplateSpec tests into a FIXME, because structural eq does not handle VarTemplates and their specs yet.

Function template specializations are different. They are all "full"
specializations. Structural equivalency does not check the body of
functions, so we cannot create conflicting function template specializations.
Thus, ODR handling strategies has nothing to do with function template
specializations. I have added this as a comment to the tests.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66951