r333082 - Fix duplicate class template definitions problem

Hans Wennborg via cfe-commits cfe-commits at lists.llvm.org
Thu May 24 03:54:29 PDT 2018


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.


More information about the cfe-commits mailing list