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.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@176999 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h
index fd1dcaa..64059a6 100644
--- a/include/clang/AST/Decl.h
+++ b/include/clang/AST/Decl.h
@@ -238,9 +238,9 @@
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 @@
/// \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 @@
/// \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 @@
// 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.
diff --git a/include/clang/AST/Type.h b/include/clang/AST/Type.h
index 10143a6..24c3459 100644
--- a/include/clang/AST/Type.h
+++ b/include/clang/AST/Type.h
@@ -1774,8 +1774,9 @@
/// \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;
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 29c1f77..5f4bb1d 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -200,6 +200,8 @@
///\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;
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index da6f03a..4b92069 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -858,49 +858,14 @@
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.
+bool NamedDecl::isLinkageValid() const {
+ if (!HasCachedLinkage)
+ return true;
- 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();
- }
-
+ return getLVForDecl(this, LVForExplicitValue).getLinkage() ==
+ Linkage(CachedLinkage);
}
Linkage NamedDecl::getLinkage() const {
@@ -1515,7 +1480,7 @@
void VarDecl::setStorageClass(StorageClass SC) {
assert(isLegalForVariable(SC));
if (getStorageClass() != SC)
- ClearLinkageCache();
+ assert(isLinkageValid());
VarDeclBits.SClass = SC;
}
@@ -1560,10 +1525,27 @@
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 @@
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 @@
void FunctionDecl::setStorageClass(StorageClass SC) {
assert(isLegalForFunction(SC));
if (getStorageClass() != SC)
- ClearLinkageCache();
+ assert(isLinkageValid());
SClass = SC;
}
@@ -2867,8 +2853,8 @@
void TagDecl::setTypedefNameForAnonDecl(TypedefNameDecl *TDD) {
TypedefNameDeclOrQualifier = TDD;
if (TypeForDecl)
- const_cast<Type*>(TypeForDecl)->ClearLinkageCache();
- ClearLinkageCache();
+ assert(TypeForDecl->isLinkageValid());
+ assert(isLinkageValid());
}
void TagDecl::startDefinition() {
diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
index ca086b1..cdd67fd 100644
--- a/lib/AST/Type.cpp
+++ b/lib/AST/Type.cpp
@@ -2236,6 +2236,14 @@
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 @@
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;
diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 6239172..09ea825 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -332,6 +332,9 @@
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 @@
return !SemaRef->ShouldWarnIfUnusedFileScopedDecl(DeclToCheck);
}
- if (D->hasExternalLinkage())
- return true;
-
return false;
}
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index dcc1537..4c6da45 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -1174,6 +1174,31 @@
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 @@
}
// 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 @@
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 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 @@
// 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 @@
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 @@
}
// 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 @@
// 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 @@
}
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 @@
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())
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 69e06ea..dc6132c 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -10594,7 +10594,7 @@
// 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) &&
diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp
index 16fd28e..3ed0465 100644
--- a/lib/Sema/SemaOverload.cpp
+++ b/lib/Sema/SemaOverload.cpp
@@ -977,13 +977,8 @@
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 @@
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 @@
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 @@
// 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 @@
// 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 @@
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.
///