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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
