[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.


================
Comment at: lib/AST/ASTImporter.cpp:4550
+      // in the "From" context, but not in the "To" context.
+      for (auto *FromField : D->fields())
+        Importer.Import(FromField);
----------------
martong wrote:
> a_sidorin wrote:
> > Importing additional fields can change the layout of the specialization. For CSA, this usually results in strange assertions hard to debug. Could you please share the results of testing of this change?
> > This change also doesn't seem to have related tests in this patch.
> TLDR; We will not create additional fields.
> 
> By the time when we import the field, we already know that the existing specialization is structurally equivalent with the new one. 
> Since a ClassTemplateSpecializationDecl is the descendant of RecordDecl, the structural equivalence check ensures that they have the exact same fields.
> When we import the field of the new spec and if there is an existing FieldDecl in the "To" context, then no new FieldDecl will be created (this is handled in `VisitFieldDecl` by first doing a lookup of existing field with the same name and type).
> This patch extends `VisitFieldDecl` in a way that we add new initializer expressions to the existing FieldDecl, if it didn't have and in the "From" context it has.
> 
> For the record, I  added a new test to make sure that a new FieldDecl will not be created during the merge.
This is the new test: `ODRViolationOfClassTemplateSpecializationsShouldBeReported`. It checks that it is not possible to add new fields to a specialization, rather an ODR violation is diagnosed.


Repository:
  rC Clang

https://reviews.llvm.org/D50451