On Wed, Apr 3, 2013 at 8:27 PM, Rafael Espindola <[email protected] > wrote:
> Author: rafael > Date: Wed Apr 3 22:27:32 2013 > New Revision: 178739 > > URL: http://llvm.org/viewvc/llvm-project?rev=178739&view=rev > Log: > Avoid computing the linkage instead of avoiding caching it. > > This mostly reverts 178733, but keeps the tests. > > I don't claim to understand how hidden sub modules work or when we need to > see > them (is that documented?), but this has the same semantics and avoids > adding > hasExternalLinkageUncached which has the same foot gun potential as the old > hasExternalLinkage. > > Last but not least, not computing linkage when it is not needed is more > efficient. > > Modified: > cfe/trunk/include/clang/AST/Decl.h > cfe/trunk/lib/AST/Decl.cpp > cfe/trunk/lib/Sema/SemaDecl.cpp > > Modified: cfe/trunk/include/clang/AST/Decl.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=178739&r1=178738&r2=178739&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/AST/Decl.h (original) > +++ cfe/trunk/include/clang/AST/Decl.h Wed Apr 3 22:27:32 2013 > @@ -219,10 +219,6 @@ public: > return getLinkage() == ExternalLinkage; > } > > - /// \brief True if this decl has external linkage. Don't cache the > linkage, > - /// because we are not finished setting up the redecl chain for the > decl. > - bool hasExternalLinkageUncached() const; > - > /// \brief Determines the visibility of this entity. > Visibility getVisibility() const { > return getLinkageAndVisibility().getVisibility(); > > Modified: cfe/trunk/lib/AST/Decl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=178739&r1=178738&r2=178739&view=diff > > ============================================================================== > --- cfe/trunk/lib/AST/Decl.cpp (original) > +++ cfe/trunk/lib/AST/Decl.cpp Wed Apr 3 22:27:32 2013 > @@ -866,10 +866,6 @@ bool NamedDecl::isLinkageValid() const { > Linkage(CachedLinkage); > } > > -bool NamedDecl::hasExternalLinkageUncached() const { > - return getLVForDecl(this, LVForExplicitValue).getLinkage() == > ExternalLinkage; > -} > - > Linkage NamedDecl::getLinkage() const { > if (HasCachedLinkage) > return Linkage(CachedLinkage); > > Modified: cfe/trunk/lib/Sema/SemaDecl.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=178739&r1=178738&r2=178739&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Apr 3 22:27:32 2013 > @@ -1605,19 +1605,6 @@ static void filterNonConflictingPrevious > if (previous.empty()) > return; > > - // If this declaration would have external linkage if it were the first > - // declaration of this name, then it may in fact be a redeclaration of > - // some hidden declaration, so include those too. We don't need to worry > - // about some previous visible declaration giving this declaration > external > - // linkage, because in that case, we'll mark this declaration as a > redecl > - // of the visible decl, and that decl will already be a redecl of the > - // hidden declaration if that's appropriate. > - // > - // Don't cache this linkage computation, because it's not yet correct: > we > - // may later give this declaration a previous declaration which changes > - // its linkage. > - bool hasExternalLinkage = decl->hasExternalLinkageUncached(); > - > LookupResult::Filter filter = previous.makeFilter(); > while (filter.hasNext()) { > NamedDecl *old = filter.next(); > @@ -1627,7 +1614,7 @@ static void filterNonConflictingPrevious > continue; > > // If either has no-external linkage, ignore the old declaration. > - if (!hasExternalLinkage || old->getLinkage() != ExternalLinkage) > + if (old->getLinkage() != ExternalLinkage || > !decl->hasExternalLinkage()) > This isn't correct; this takes us back to caching the linkage of 'decl' before we set a previous declaration. Testcase attached; build with: clang++ main.cc -I$PWD/mod -fmodules -fcxx-modules
r178739.tar
Description: Unix tar archive
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
