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