Thanks a lot! Gabor
On Thu, May 24, 2018 at 12:54 PM Hans Wennborg <h...@chromium.org> wrote: > On Wed, May 23, 2018 at 3:53 PM, Gabor Marton via cfe-commits > <cfe-commits@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. >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
