r333082 - Fix duplicate class template definitions problem

[Gábor Márton via cfe-commits]

Thanks a lot!

Gabor

On Thu, May 24, 2018 at 12:54 PM Hans Wennborg <hans at chromium.org> wrote:

> On Wed, May 23, 2018 at 3:53 PM, Gabor Marton via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
> > Author: martong
> > Date: Wed May 23 06:53:36 2018
> > New Revision: 333082
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=333082&view=rev
> > Log:
> > Fix duplicate class template definitions problem
> >
> > Summary:
> > We fail to import a `ClassTemplateDecl` if the "To" context already
> > contains a definition and then a forward decl.  This is because
> > `localUncachedLookup` does not find the definition.  This is not a
> > lookup error, the parser behaves differently than assumed in the
> > importer code.  A `DeclContext` contains one DenseMap (`LookupPtr`)
> > which maps names to lists.  The list is a special list `StoredDeclsList`
> > which is optimized to have one element.  During building the initial
> > AST, the parser first adds the definition to the `DeclContext`.  Then
> > during parsing the second declaration (the forward decl) the parser
> > again calls `DeclContext::addDecl` but that will not add a new element
> > to the `StoredDeclsList` rarther it simply overwrites the old element
> > with the most recent one.  This patch fixes the error by finding the
> > definition in the redecl chain.  Added tests for the same issue with
> > `CXXRecordDecl` and with `ClassTemplateSpecializationDecl`.  These tests
> > pass and they pass because in `VisitRecordDecl` and in
> > `VisitClassTemplateSpecializationDecl` we already use
> > `D->getDefinition()` after the lookup.
> >
> > Reviewers: a.sidorin, xazax.hun, szepet
> >
> > Subscribers: rnkovacs, dkrupp, cfe-commits
> >
> > Differential Revision: https://reviews.llvm.org/D46950
>
> [..]
>
> > +TEST_P(ASTImporterTestBase,
> > +       ImportDefinitionOfClassIfThereIsAnExistingFwdDeclAndDefinition) {
> > +  Decl *ToTU = getToTuDecl(
> > +      R"(
> > +      struct B {
> > +        void f();
> > +      };
> > +
> > +      struct B;
> > +      )",
> > +      Lang_CXX);
> > +  ASSERT_EQ(2u, DeclCounter<CXXRecordDecl>().match(
> > +                    ToTU,
> cxxRecordDecl(hasParent(translationUnitDecl()))));
>
> This doesn't hold when targeting Windows, as Sema will add an implicit
> declaration of type_info, causing the matcher to return 3 instead of
> 2.
>
> I've committed r333170 to fix.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180524/c5864fde/attachment.html>