r176999 - Avoid computing the linkage too early. Don't invalidate it.
Rafael Espindola
rafael.espindola at gmail.com
Wed Mar 13 20:07:35 PDT 2013
Author: rafael
Date: Wed Mar 13 22:07:35 2013
New Revision: 176999
URL: http://llvm.org/viewvc/llvm-project?rev=176999&view=rev
Log:
Avoid computing the linkage too early. Don't invalidate it.
Before this patch we would compute the linkage lazily and cache it. When the
AST was modified in ways that could change the value, we would invalidate the
cache.
That was fairly brittle, since any code could ask for the a linkage before
the correct value was available.
We should change the API to one where the linkage is computed explicitly and
trying to get it when it is not available asserts.
This patch is a first step in that direction. We still compute the linkage
lazily, but instead of invalidating a cache, we assert that the AST
modifications didn't change the result.
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=176999&r1=176998&r2=176999&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Wed Mar 13 22:07:35 2013
@@ -238,9 +238,9 @@ public:
Optional<Visibility>
getExplicitVisibility(ExplicitVisibilityKind kind) const;
- /// \brief Clear the linkage cache in response to a change
- /// to the declaration.
- void ClearLinkageCache();
+ /// \brief True if the computed linkage is valid. Used for consistency
+ /// checking. Should always return true.
+ bool isLinkageValid() const;
/// \brief Looks through UsingDecls and ObjCCompatibleAliasDecls for
/// the underlying named decl.
@@ -822,9 +822,7 @@ public:
/// \brief Determines whether this variable is a variable with
/// external, C linkage.
- bool isExternC() const {
- return getLanguageLinkage() == CLanguageLinkage;
- }
+ bool isExternC() const;
/// isLocalVarDecl - Returns true for local variable declarations
/// other than parameters. Note that this includes static variables
@@ -1713,9 +1711,7 @@ public:
/// \brief Determines whether this function is a function with
/// external, C linkage.
- bool isExternC() const {
- return getLanguageLinkage() == CLanguageLinkage;
- }
+ bool isExternC() const;
/// \brief Determines whether this is a global function.
bool isGlobal() const;
@@ -3304,7 +3300,7 @@ void Redeclarable<decl_type>::setPreviou
// First one will point to this one as latest.
First->RedeclLink = LatestDeclLink(static_cast<decl_type*>(this));
if (NamedDecl *ND = dyn_cast<NamedDecl>(static_cast<decl_type*>(this)))
- ND->ClearLinkageCache();
+ assert(ND->isLinkageValid());
}
// Inline function definitions.
Modified: cfe/trunk/include/clang/AST/Type.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=176999&r1=176998&r2=176999&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Wed Mar 13 22:07:35 2013
@@ -1774,8 +1774,9 @@ public:
/// \brief Determine the linkage and visibility of this type.
LinkageInfo getLinkageAndVisibility() const;
- /// \brief Note that the linkage is no longer known.
- void ClearLinkageCache();
+ /// \brief True if the computed linkage is valid. Used for consistency
+ /// checking. Should always return true.
+ bool isLinkageValid() const;
const char *getTypeClassName() const;
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=176999&r1=176998&r2=176999&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Mar 13 22:07:35 2013
@@ -200,6 +200,8 @@ class Sema {
///\brief Whether Sema has generated a multiplexer and has to delete it.
bool isMultiplexExternalSource;
+ static bool mightHaveNonExternalLinkage(const DeclaratorDecl *FD);
+
public:
typedef OpaquePtr<DeclGroupRef> DeclGroupPtrTy;
typedef OpaquePtr<TemplateName> TemplateTy;
Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=176999&r1=176998&r2=176999&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Wed Mar 13 22:07:35 2013
@@ -858,49 +858,14 @@ static LinkageInfo getLVForClassMember(c
return LV;
}
-static void clearLinkageForClass(const CXXRecordDecl *record) {
- for (CXXRecordDecl::decl_iterator
- i = record->decls_begin(), e = record->decls_end(); i != e; ++i) {
- Decl *child = *i;
- if (isa<NamedDecl>(child))
- cast<NamedDecl>(child)->ClearLinkageCache();
- }
-}
-
void NamedDecl::anchor() { }
-void NamedDecl::ClearLinkageCache() {
- // Note that we can't skip clearing the linkage of children just
- // because the parent doesn't have cached linkage: we don't cache
- // when computing linkage for parent contexts.
-
- HasCachedLinkage = 0;
-
- // If we're changing the linkage of a class, we need to reset the
- // linkage of child declarations, too.
- if (const CXXRecordDecl *record = dyn_cast<CXXRecordDecl>(this))
- clearLinkageForClass(record);
-
- if (ClassTemplateDecl *temp = dyn_cast<ClassTemplateDecl>(this)) {
- // Clear linkage for the template pattern.
- CXXRecordDecl *record = temp->getTemplatedDecl();
- record->HasCachedLinkage = 0;
- clearLinkageForClass(record);
-
- // We need to clear linkage for specializations, too.
- for (ClassTemplateDecl::spec_iterator
- i = temp->spec_begin(), e = temp->spec_end(); i != e; ++i)
- i->ClearLinkageCache();
- }
-
- // Clear cached linkage for function template decls, too.
- if (FunctionTemplateDecl *temp = dyn_cast<FunctionTemplateDecl>(this)) {
- temp->getTemplatedDecl()->ClearLinkageCache();
- for (FunctionTemplateDecl::spec_iterator
- i = temp->spec_begin(), e = temp->spec_end(); i != e; ++i)
- i->ClearLinkageCache();
- }
-
+bool NamedDecl::isLinkageValid() const {
+ if (!HasCachedLinkage)
+ return true;
+
+ return getLVForDecl(this, LVForExplicitValue).getLinkage() ==
+ Linkage(CachedLinkage);
}
Linkage NamedDecl::getLinkage() const {
@@ -1515,7 +1480,7 @@ VarDecl *VarDecl::CreateDeserialized(AST
void VarDecl::setStorageClass(StorageClass SC) {
assert(isLegalForVariable(SC));
if (getStorageClass() != SC)
- ClearLinkageCache();
+ assert(isLinkageValid());
VarDeclBits.SClass = SC;
}
@@ -1560,10 +1525,27 @@ static LanguageLinkage getLanguageLinkag
return CXXLanguageLinkage;
}
+template<typename T>
+static bool isExternCTemplate(const T &D) {
+ // Since the context is ignored for class members, they can only have C++
+ // language linkage or no language linkage.
+ const DeclContext *DC = D.getDeclContext();
+ if (DC->isRecord()) {
+ assert(D.getASTContext().getLangOpts().CPlusPlus);
+ return false;
+ }
+
+ return D.getLanguageLinkage() == CLanguageLinkage;
+}
+
LanguageLinkage VarDecl::getLanguageLinkage() const {
return getLanguageLinkageTemplate(*this);
}
+bool VarDecl::isExternC() const {
+ return isExternCTemplate(*this);
+}
+
VarDecl *VarDecl::getCanonicalDecl() {
return getFirstDeclaration();
}
@@ -2077,6 +2059,10 @@ LanguageLinkage FunctionDecl::getLanguag
return getLanguageLinkageTemplate(*this);
}
+bool FunctionDecl::isExternC() const {
+ return isExternCTemplate(*this);
+}
+
bool FunctionDecl::isGlobal() const {
if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(this))
return Method->isStatic();
@@ -2129,7 +2115,7 @@ FunctionDecl *FunctionDecl::getCanonical
void FunctionDecl::setStorageClass(StorageClass SC) {
assert(isLegalForFunction(SC));
if (getStorageClass() != SC)
- ClearLinkageCache();
+ assert(isLinkageValid());
SClass = SC;
}
@@ -2867,8 +2853,8 @@ TagDecl* TagDecl::getCanonicalDecl() {
void TagDecl::setTypedefNameForAnonDecl(TypedefNameDecl *TDD) {
TypedefNameDeclOrQualifier = TDD;
if (TypeForDecl)
- const_cast<Type*>(TypeForDecl)->ClearLinkageCache();
- ClearLinkageCache();
+ assert(TypeForDecl->isLinkageValid());
+ assert(isLinkageValid());
}
void TagDecl::startDefinition() {
Modified: cfe/trunk/lib/AST/Type.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=176999&r1=176998&r2=176999&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Wed Mar 13 22:07:35 2013
@@ -2236,6 +2236,14 @@ static LinkageInfo computeLinkageInfo(Qu
return computeLinkageInfo(T.getTypePtr());
}
+bool Type::isLinkageValid() const {
+ if (!TypeBits.isCacheValid())
+ return true;
+
+ return computeLinkageInfo(getCanonicalTypeInternal()).getLinkage() ==
+ TypeBits.getLinkage();
+}
+
LinkageInfo Type::getLinkageAndVisibility() const {
if (!isCanonicalUnqualified())
return computeLinkageInfo(getCanonicalTypeInternal());
@@ -2245,12 +2253,6 @@ LinkageInfo Type::getLinkageAndVisibilit
return LV;
}
-void Type::ClearLinkageCache() {
- TypeBits.CacheValid = false;
- if (QualType(this, 0) != CanonicalType)
- CanonicalType->TypeBits.CacheValid = false;
-}
-
Qualifiers::ObjCLifetime Type::getObjCARCImplicitLifetime() const {
if (isObjCARCImplicitlyUnretainedType())
return Qualifiers::OCL_ExplicitNone;
Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=176999&r1=176998&r2=176999&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Wed Mar 13 22:07:35 2013
@@ -332,6 +332,9 @@ static bool ShouldRemoveFromUnused(Sema
if (D->getMostRecentDecl()->isUsed())
return true;
+ if (D->hasExternalLinkage())
+ return true;
+
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
// UnusedFileScopedDecls stores the first declaration.
// The declaration may have become definition so check again.
@@ -360,9 +363,6 @@ static bool ShouldRemoveFromUnused(Sema
return !SemaRef->ShouldWarnIfUnusedFileScopedDecl(DeclToCheck);
}
- if (D->hasExternalLinkage())
- return true;
-
return false;
}
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=176999&r1=176998&r2=176999&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Mar 13 22:07:35 2013
@@ -1174,6 +1174,31 @@ static bool IsDisallowedCopyOrAssign(con
return false;
}
+// We need this to handle
+//
+// typedef struct {
+// void *foo() { return 0; }
+// } A;
+//
+// When we see foo we don't know if after the typedef we will get 'A' or '*A'
+// for example. If 'A', foo will have external linkage. If we have '*A',
+// foo will have no linkage. Since we can't know untill we get to the end
+// of the typedef, this function finds out if D might have non external linkage.
+// Callers should verify at the end of the TU if it D has external linkage or
+// not.
+bool Sema::mightHaveNonExternalLinkage(const DeclaratorDecl *D) {
+ const DeclContext *DC = D->getDeclContext();
+ while (!DC->isTranslationUnit()) {
+ if (const RecordDecl *RD = dyn_cast<RecordDecl>(DC)){
+ if (!RD->hasNameForLinkage())
+ return true;
+ }
+ DC = DC->getParent();
+ }
+
+ return !D->hasExternalLinkage();
+}
+
bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const {
assert(D);
@@ -1223,10 +1248,7 @@ bool Sema::ShouldWarnIfUnusedFileScopedD
}
// Only warn for unused decls internal to the translation unit.
- if (D->hasExternalLinkage())
- return false;
-
- return true;
+ return mightHaveNonExternalLinkage(D);
}
void Sema::MarkUnusedFileScopedDecl(const DeclaratorDecl *D) {
@@ -2886,7 +2908,7 @@ void Sema::MergeVarDecl(VarDecl *New, Lo
return New->setInvalidDecl();
}
if (Old->hasExternalStorage() &&
- !New->hasLinkage() && New->isLocalVarDecl()) {
+ New->isLocalVarDecl() && !New->hasLinkage()) {
Diag(New->getLocation(), diag::err_non_extern_extern) << New->getDeclName();
Diag(Old->getLocation(), diag::note_previous_definition);
return New->setInvalidDecl();
@@ -4583,6 +4605,26 @@ static void checkAttributesAfterMerging(
}
}
+static bool shouldConsiderLinkage(const VarDecl *VD) {
+ const DeclContext *DC = VD->getDeclContext()->getRedeclContext();
+ if (DC->isFunctionOrMethod())
+ return VD->hasExternalStorageAsWritten();
+ if (DC->isFileContext())
+ return true;
+ if (DC->isRecord())
+ return false;
+ llvm_unreachable("Unexpected context");
+}
+
+static bool shouldConsiderLinkage(const FunctionDecl *FD) {
+ const DeclContext *DC = FD->getDeclContext()->getRedeclContext();
+ if (DC->isFileContext() || DC->isFunctionOrMethod())
+ return true;
+ if (DC->isRecord())
+ return false;
+ llvm_unreachable("Unexpected context");
+}
+
NamedDecl*
Sema::ActOnVariableDeclarator(Scope *S, Declarator &D, DeclContext *DC,
TypeSourceInfo *TInfo, LookupResult &Previous,
@@ -4861,7 +4903,7 @@ Sema::ActOnVariableDeclarator(Scope *S,
// Don't consider existing declarations that are in a different
// scope and are out-of-semantic-context declarations (if the new
// declaration has linkage).
- FilterLookupForScope(Previous, DC, S, NewVD->hasLinkage(),
+ FilterLookupForScope(Previous, DC, S, shouldConsiderLinkage(NewVD),
isExplicitSpecialization);
if (!getLangOpts().CPlusPlus) {
@@ -5004,8 +5046,28 @@ void Sema::CheckShadow(Scope *S, VarDecl
template<typename T>
static bool mayConflictWithNonVisibleExternC(const T *ND) {
- return ND->isExternC() ||
- ND->getDeclContext()->getRedeclContext()->isTranslationUnit();
+ const DeclContext *DC = ND->getDeclContext();
+ if (DC->getRedeclContext()->isTranslationUnit())
+ return true;
+
+ // We know that is the first decl we see, other than function local
+ // extern C ones. If this is C++ and the decl is not in a extern C context
+ // it cannot have C language linkage. Avoid calling isExternC in that case.
+ // We need to this because of code like
+ //
+ // namespace { struct bar {}; }
+ // auto foo = bar();
+ //
+ // This code runs before the init of foo is set, and therefore before
+ // the type of foo is known. Not knowing the type we cannot know its linkage
+ // unless it is in an extern C block.
+ if (!DC->isExternCContext()) {
+ const ASTContext &Context = ND->getASTContext();
+ if (Context.getLangOpts().CPlusPlus)
+ return false;
+ }
+
+ return ND->isExternC();
}
/// \brief Perform semantic checking on a newly-created variable
@@ -6003,7 +6065,7 @@ Sema::ActOnFunctionDeclarator(Scope *S,
}
// Filter out previous declarations that don't match the scope.
- FilterLookupForScope(Previous, DC, S, NewFD->hasLinkage(),
+ FilterLookupForScope(Previous, DC, S, shouldConsiderLinkage(NewFD),
isExplicitSpecialization ||
isFunctionTemplateSpecialization);
@@ -6390,7 +6452,7 @@ Sema::ActOnFunctionDeclarator(Scope *S,
// If there's a #pragma GCC visibility in scope, and this isn't a class
// member, set the visibility of this function.
- if (NewFD->hasExternalLinkage() && !DC->isRecord())
+ if (!DC->isRecord() && NewFD->hasExternalLinkage())
AddPushedVisibilityAttribute(NewFD);
// If there's a #pragma clang arc_cf_code_audited in scope, consider
@@ -7152,7 +7214,7 @@ void Sema::AddInitializerToDecl(Decl *Re
}
VDecl->setTypeSourceInfo(DeducedType);
VDecl->setType(DeducedType->getType());
- VDecl->ClearLinkageCache();
+ assert(VDecl->isLinkageValid());
// In ARC, infer lifetime.
if (getLangOpts().ObjCAutoRefCount && inferObjCARCLifetime(VDecl))
@@ -7897,7 +7959,7 @@ Sema::FinalizeDeclaration(Decl *ThisDecl
const DeclContext *DC = VD->getDeclContext();
// If there's a #pragma GCC visibility in scope, and this isn't a class
// member, set the visibility of this variable.
- if (VD->hasExternalLinkage() && !DC->isRecord())
+ if (!DC->isRecord() && VD->hasExternalLinkage())
AddPushedVisibilityAttribute(VD);
if (VD->isFileVarDecl())
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=176999&r1=176998&r2=176999&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Mar 13 22:07:35 2013
@@ -10594,7 +10594,7 @@ void Sema::MarkFunctionReferenced(Source
// Keep track of used but undefined functions.
if (!Func->isDefined()) {
- if (Func->getLinkage() != ExternalLinkage)
+ if (mightHaveNonExternalLinkage(Func))
UndefinedButUsed.insert(std::make_pair(Func->getCanonicalDecl(), Loc));
else if (Func->getMostRecentDecl()->isInlined() &&
(LangOpts.CPlusPlus || !LangOpts.GNUInline) &&
Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=176999&r1=176998&r2=176999&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Wed Mar 13 22:07:35 2013
@@ -977,13 +977,8 @@ static bool canBeOverloaded(const Functi
return true;
}
-bool Sema::IsOverload(FunctionDecl *New, FunctionDecl *Old,
- bool UseUsingDeclRules) {
- // If both of the functions are extern "C", then they are not
- // overloads.
- if (!canBeOverloaded(*Old) && !canBeOverloaded(*New))
- return false;
-
+static bool shouldTryToOverload(Sema &S, FunctionDecl *New, FunctionDecl *Old,
+ bool UseUsingDeclRules) {
FunctionTemplateDecl *OldTemplate = Old->getDescribedFunctionTemplate();
FunctionTemplateDecl *NewTemplate = New->getDescribedFunctionTemplate();
@@ -994,8 +989,8 @@ bool Sema::IsOverload(FunctionDecl *New,
return true;
// Is the function New an overload of the function Old?
- QualType OldQType = Context.getCanonicalType(Old->getType());
- QualType NewQType = Context.getCanonicalType(New->getType());
+ QualType OldQType = S.Context.getCanonicalType(Old->getType());
+ QualType NewQType = S.Context.getCanonicalType(New->getType());
// Compare the signatures (C++ 1.3.10) of the two functions to
// determine whether they are overloads. If we find any mismatch
@@ -1016,7 +1011,7 @@ bool Sema::IsOverload(FunctionDecl *New,
if (OldQType != NewQType &&
(OldType->getNumArgs() != NewType->getNumArgs() ||
OldType->isVariadic() != NewType->isVariadic() ||
- !FunctionArgTypesAreEqual(OldType, NewType)))
+ !S.FunctionArgTypesAreEqual(OldType, NewType)))
return true;
// C++ [temp.over.link]p4:
@@ -1032,9 +1027,9 @@ bool Sema::IsOverload(FunctionDecl *New,
// However, we don't consider either of these when deciding whether
// a member introduced by a shadow declaration is hidden.
if (!UseUsingDeclRules && NewTemplate &&
- (!TemplateParameterListsAreEqual(NewTemplate->getTemplateParameters(),
- OldTemplate->getTemplateParameters(),
- false, TPL_TemplateMatch) ||
+ (!S.TemplateParameterListsAreEqual(NewTemplate->getTemplateParameters(),
+ OldTemplate->getTemplateParameters(),
+ false, S.TPL_TemplateMatch) ||
OldType->getResultType() != NewType->getResultType()))
return true;
@@ -1060,9 +1055,9 @@ bool Sema::IsOverload(FunctionDecl *New,
// declarations with the same name, the same parameter-type-list, and
// the same template parameter lists cannot be overloaded if any of
// them, but not all, have a ref-qualifier (8.3.5).
- Diag(NewMethod->getLocation(), diag::err_ref_qualifier_overload)
+ S.Diag(NewMethod->getLocation(), diag::err_ref_qualifier_overload)
<< NewMethod->getRefQualifier() << OldMethod->getRefQualifier();
- Diag(OldMethod->getLocation(), diag::note_previous_declaration);
+ S.Diag(OldMethod->getLocation(), diag::note_previous_declaration);
}
return true;
}
@@ -1082,6 +1077,19 @@ bool Sema::IsOverload(FunctionDecl *New,
return false;
}
+bool Sema::IsOverload(FunctionDecl *New, FunctionDecl *Old,
+ bool UseUsingDeclRules) {
+ if (!shouldTryToOverload(*this, New, Old, UseUsingDeclRules))
+ return false;
+
+ // If both of the functions are extern "C", then they are not
+ // overloads.
+ if (!canBeOverloaded(*Old) && !canBeOverloaded(*New))
+ return false;
+
+ return true;
+}
+
/// \brief Checks availability of the function depending on the current
/// function context. Inside an unavailable function, unavailability is ignored.
///
More information about the cfe-commits
mailing list