a_sidorin 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:
> 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.
Thank you for the explanation. However, I find the comment very misleading. It
tells:
```
// Check and merge those fields which have been instantiated
// in the "From" context, but not in the "To" context.
```
Would it be correct to change it to "Import field initializers that are still
not instantiated", or do I still misunderstand something?
Repository:
rC Clang
https://reviews.llvm.org/D50451
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
