<div dir="ltr">On Wed, Apr 3, 2013 at 8:27 PM, Rafael Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank" class="cremed">rafael.espindola@gmail.com</a>></span> wrote:<br><div class="gmail_extra">
<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rafael<br>
Date: Wed Apr  3 22:27:32 2013<br>
New Revision: 178739<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=178739&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=178739&view=rev</a><br>
Log:<br>
Avoid computing the linkage instead of avoiding caching it.<br>
<br>
This mostly reverts 178733, but keeps the tests.<br>
<br>
I don't claim to understand how hidden sub modules work or when we need to see<br>
them (is that documented?), but this has the same semantics and avoids adding<br>
hasExternalLinkageUncached which has the same foot gun potential as the old<br>
hasExternalLinkage.<br>
<br>
Last but not least, not computing linkage when it is not needed is more<br>
efficient.<br>
<br>
Modified:<br>
    cfe/trunk/include/clang/AST/Decl.h<br>
    cfe/trunk/lib/AST/Decl.cpp<br>
    cfe/trunk/lib/Sema/SemaDecl.cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/Decl.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=178739&r1=178738&r2=178739&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=178739&r1=178738&r2=178739&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/include/clang/AST/Decl.h (original)<br>
+++ cfe/trunk/include/clang/AST/Decl.h Wed Apr  3 22:27:32 2013<br>
@@ -219,10 +219,6 @@ public:<br>
     return getLinkage() == ExternalLinkage;<br>
   }<br>
<br>
-  /// \brief True if this decl has external linkage. Don't cache the linkage,<br>
-  /// because we are not finished setting up the redecl chain for the decl.<br>
-  bool hasExternalLinkageUncached() const;<br>
-<br>
   /// \brief Determines the visibility of this entity.<br>
   Visibility getVisibility() const {<br>
     return getLinkageAndVisibility().getVisibility();<br>
<br>
Modified: cfe/trunk/lib/AST/Decl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=178739&r1=178738&r2=178739&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=178739&r1=178738&r2=178739&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/AST/Decl.cpp (original)<br>
+++ cfe/trunk/lib/AST/Decl.cpp Wed Apr  3 22:27:32 2013<br>
@@ -866,10 +866,6 @@ bool NamedDecl::isLinkageValid() const {<br>
     Linkage(CachedLinkage);<br>
 }<br>
<br>
-bool NamedDecl::hasExternalLinkageUncached() const {<br>
-  return getLVForDecl(this, LVForExplicitValue).getLinkage() == ExternalLinkage;<br>
-}<br>
-<br>
 Linkage NamedDecl::getLinkage() const {<br>
   if (HasCachedLinkage)<br>
     return Linkage(CachedLinkage);<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=178739&r1=178738&r2=178739&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=178739&r1=178738&r2=178739&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Apr  3 22:27:32 2013<br>
@@ -1605,19 +1605,6 @@ static void filterNonConflictingPrevious<br>
   if (previous.empty())<br>
     return;<br>
<br>
-  // If this declaration would have external linkage if it were the first<br>
-  // declaration of this name, then it may in fact be a redeclaration of<br>
-  // some hidden declaration, so include those too. We don't need to worry<br>
-  // about some previous visible declaration giving this declaration external<br>
-  // linkage, because in that case, we'll mark this declaration as a redecl<br>
-  // of the visible decl, and that decl will already be a redecl of the<br>
-  // hidden declaration if that's appropriate.<br>
-  //<br>
-  // Don't cache this linkage computation, because it's not yet correct: we<br>
-  // may later give this declaration a previous declaration which changes<br>
-  // its linkage.<br>
-  bool hasExternalLinkage = decl->hasExternalLinkageUncached();<br>
-<br>
   LookupResult::Filter filter = previous.makeFilter();<br>
   while (filter.hasNext()) {<br>
     NamedDecl *old = filter.next();<br>
@@ -1627,7 +1614,7 @@ static void filterNonConflictingPrevious<br>
       continue;<br>
<br>
     // If either has no-external linkage, ignore the old declaration.<br>
-    if (!hasExternalLinkage || old->getLinkage() != ExternalLinkage)<br>
+    if (old->getLinkage() != ExternalLinkage || !decl->hasExternalLinkage())<br></blockquote><div><br></div><div style>This isn't correct; this takes us back to caching the linkage of 'decl' before we set a previous declaration. Testcase attached; build with:</div>
<div style><br></div><div style>clang++ main.cc -I$PWD/mod -fmodules -fcxx-modules</div></div></div></div>