[cfe-commits] r172305 - in /cfe/trunk: include/clang/AST/Decl.h include/clang/AST/DeclBase.h include/clang/AST/Type.h lib/AST/ASTContext.cpp lib/AST/Decl.cpp lib/AST/Type.cpp lib/Sema/SemaAttr.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclAttr.cpp test/CodeGenCXX/visibility-inlines-hidden.cpp test/CodeGenCXX/visibility.cpp
Rafael Espindola
rafael.espindola at gmail.com
Fri Jan 11 22:42:30 PST 2013
Author: rafael
Date: Sat Jan 12 00:42:30 2013
New Revision: 172305
URL: http://llvm.org/viewvc/llvm-project?rev=172305&view=rev
Log:
Disable caching of visibility.
The testcase in pr14929 shows that this is extremely hard to do. If we choose
to apply the attribute, that causes the visibility of some decls to change and
that can happen really late (during codegen).
Current gcc warns and ignores the attribute in this testcase with a warning.
This suggest that the correct solution is to find a point in the compilation
where we can compute the visibility and
* assert it was never computed before
* reject any attempts to compute it again in the future (with warnings).
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/SemaAttr.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/CodeGenCXX/visibility-inlines-hidden.cpp
cfe/trunk/test/CodeGenCXX/visibility.cpp
Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=172305&r1=172304&r2=172305&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Sat Jan 12 00:42:30 2013
@@ -109,6 +109,7 @@
private:
NamedDecl *getUnderlyingDeclImpl();
+ void verifyLinkage() const;
protected:
NamedDecl(Kind DK, DeclContext *DC, SourceLocation L, DeclarationName N)
@@ -228,12 +229,6 @@
"Enum truncated!");
}
- bool operator==(const LinkageInfo &Other) {
- return linkage_ == Other.linkage_ &&
- visibility_ == Other.visibility_ &&
- explicit_ == Other.explicit_;
- }
-
static LinkageInfo external() {
return LinkageInfo();
}
@@ -329,7 +324,7 @@
/// \brief Clear the linkage cache in response to a change
/// to the declaration.
- void ClearLVCache();
+ void ClearLinkageCache();
/// \brief Looks through UsingDecls and ObjCCompatibleAliasDecls for
/// the underlying named decl.
@@ -3337,7 +3332,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->ClearLVCache();
+ ND->ClearLinkageCache();
}
// Inline function definitions.
Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=172305&r1=172304&r2=172305&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Sat Jan 12 00:42:30 2013
@@ -242,7 +242,7 @@
SourceLocation Loc;
/// DeclKind - This indicates which class this is.
- unsigned DeclKind : 6;
+ unsigned DeclKind : 8;
/// InvalidDecl - This indicates a semantic error occurred.
unsigned InvalidDecl : 1;
@@ -284,16 +284,15 @@
/// IdentifierNamespace - This specifies what IDNS_* namespace this lives in.
unsigned IdentifierNamespace : 12;
- /// These fields are only valid for NamedDecls subclasses.
+ /// \brief Whether the \c CachedLinkage field is active.
+ ///
+ /// This field is only valid for NamedDecls subclasses.
+ mutable unsigned HasCachedLinkage : 1;
+
+ /// \brief If \c HasCachedLinkage, the linkage of this declaration.
///
- /// \brief Nonzero if the cache (i.e. the bitfields here starting
- /// with 'Cache') is valid. If so, then this is a
- /// LangOptions::VisibilityMode+1.
- mutable unsigned CacheValidAndVisibility : 2;
- /// \brief the linkage of this declaration.
+ /// This field is only valid for NamedDecls subclasses.
mutable unsigned CachedLinkage : 2;
- /// \brief true if the visibility is explicit.
- mutable unsigned CachedVisibilityExplicit : 1;
friend class ASTDeclWriter;
friend class ASTDeclReader;
@@ -310,7 +309,7 @@
HasAttrs(false), Implicit(false), Used(false), Referenced(false),
Access(AS_none), FromASTFile(0), Hidden(0),
IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
- CacheValidAndVisibility(0)
+ HasCachedLinkage(0)
{
if (StatisticsEnabled) add(DK);
}
@@ -320,7 +319,7 @@
HasAttrs(false), Implicit(false), Used(false), Referenced(false),
Access(AS_none), FromASTFile(0), Hidden(0),
IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
- CacheValidAndVisibility(0)
+ HasCachedLinkage(0)
{
if (StatisticsEnabled) add(DK);
}
Modified: cfe/trunk/include/clang/AST/Type.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=172305&r1=172304&r2=172305&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Sat Jan 12 00:42:30 2013
@@ -1781,7 +1781,7 @@
std::pair<Linkage,Visibility> getLinkageAndVisibility() const;
/// \brief Note that the linkage is no longer known.
- void ClearLVCache();
+ void ClearLinkageCache();
const char *getTypeClassName() const;
Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=172305&r1=172304&r2=172305&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Sat Jan 12 00:42:30 2013
@@ -958,7 +958,6 @@
"Already noted what static data member was instantiated from");
InstantiatedFromStaticDataMember[Inst]
= new (*this) MemberSpecializationInfo(Tmpl, TSK, PointOfInstantiation);
- Inst->ClearLVCache();
}
FunctionDecl *ASTContext::getClassScopeSpecializationPattern(
Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=172305&r1=172304&r2=172305&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Sat Jan 12 00:42:30 2013
@@ -104,14 +104,8 @@
return LV;
}
-/// Compute the linkage and visibility for the given declaration.
-static LinkageInfo computeLVForDecl(const NamedDecl *D, bool OnlyTemplate);
-
-static LinkageInfo getLVForDecl(const NamedDecl *D, bool OnlyTemplate) {
- if (!OnlyTemplate)
- return D->getLinkageAndVisibility();
- return computeLVForDecl(D, OnlyTemplate);
-}
+/// getLVForDecl - Get the linkage and visibility for the given declaration.
+static LinkageInfo getLVForDecl(const NamedDecl *D, bool OnlyTemplate);
/// \brief Get the most restrictive linkage for the types and
/// declarations in the given template argument list.
@@ -575,18 +569,18 @@
i = record->decls_begin(), e = record->decls_end(); i != e; ++i) {
Decl *child = *i;
if (isa<NamedDecl>(child))
- cast<NamedDecl>(child)->ClearLVCache();
+ cast<NamedDecl>(child)->ClearLinkageCache();
}
}
void NamedDecl::anchor() { }
-void NamedDecl::ClearLVCache() {
+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.
- CacheValidAndVisibility = 0;
+ HasCachedLinkage = 0;
// If we're changing the linkage of a class, we need to reset the
// linkage of child declarations, too.
@@ -597,51 +591,66 @@
dyn_cast<ClassTemplateDecl>(const_cast<NamedDecl*>(this))) {
// Clear linkage for the template pattern.
CXXRecordDecl *record = temp->getTemplatedDecl();
- record->CacheValidAndVisibility = 0;
+ 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->ClearLVCache();
+ i->ClearLinkageCache();
}
// Clear cached linkage for function template decls, too.
if (FunctionTemplateDecl *temp =
dyn_cast<FunctionTemplateDecl>(const_cast<NamedDecl*>(this))) {
- temp->getTemplatedDecl()->ClearLVCache();
+ temp->getTemplatedDecl()->ClearLinkageCache();
for (FunctionTemplateDecl::spec_iterator
i = temp->spec_begin(), e = temp->spec_end(); i != e; ++i)
- i->ClearLVCache();
+ i->ClearLinkageCache();
}
}
Linkage NamedDecl::getLinkage() const {
- return getLinkageAndVisibility().linkage();
+ if (HasCachedLinkage) {
+ assert(Linkage(CachedLinkage) ==
+ getLVForDecl(this, true).linkage());
+ return Linkage(CachedLinkage);
+ }
+
+ CachedLinkage = getLVForDecl(this, true).linkage();
+ HasCachedLinkage = 1;
+
+#ifndef NDEBUG
+ verifyLinkage();
+#endif
+
+ return Linkage(CachedLinkage);
}
LinkageInfo NamedDecl::getLinkageAndVisibility() const {
- if (CacheValidAndVisibility) {
- Linkage L = static_cast<Linkage>(CachedLinkage);
- Visibility V = static_cast<Visibility>(CacheValidAndVisibility - 1);
- bool Explicit = CachedVisibilityExplicit;
- LinkageInfo LV(L, V, Explicit);
- assert(LV == computeLVForDecl(this, false));
- return LV;
+ LinkageInfo LI = getLVForDecl(this, false);
+ if (HasCachedLinkage) {
+ assert(Linkage(CachedLinkage) == LI.linkage());
+ return LI;
}
- LinkageInfo LV = computeLVForDecl(this, false);
- CachedLinkage = LV.linkage();
- CacheValidAndVisibility = LV.visibility() + 1;
- CachedVisibilityExplicit = LV.visibilityExplicit();
+ HasCachedLinkage = 1;
+ CachedLinkage = LI.linkage();
#ifndef NDEBUG
+ verifyLinkage();
+#endif
+
+ return LI;
+}
+
+void NamedDecl::verifyLinkage() const {
// In C (because of gnu inline) and in c++ with microsoft extensions an
// static can follow an extern, so we can have two decls with different
// linkages.
const LangOptions &Opts = getASTContext().getLangOpts();
if (!Opts.CPlusPlus || Opts.MicrosoftExt)
- return LV;
+ return;
// We have just computed the linkage for this decl. By induction we know
// that all other computed linkages match, check that the one we just computed
@@ -651,15 +660,12 @@
NamedDecl *T = cast<NamedDecl>(*I);
if (T == this)
continue;
- if (T->CacheValidAndVisibility != 0) {
+ if (T->HasCachedLinkage != 0) {
D = T;
break;
}
}
assert(!D || D->CachedLinkage == CachedLinkage);
-#endif
-
- return LV;
}
llvm::Optional<Visibility> NamedDecl::getExplicitVisibility() const {
@@ -723,7 +729,7 @@
return llvm::Optional<Visibility>();
}
-static LinkageInfo computeLVForDecl(const NamedDecl *D, bool OnlyTemplate) {
+static LinkageInfo getLVForDecl(const NamedDecl *D, bool OnlyTemplate) {
// Objective-C: treat all Objective-C declarations as having external
// linkage.
switch (D->getKind()) {
@@ -1192,7 +1198,7 @@
void VarDecl::setStorageClass(StorageClass SC) {
assert(isLegalForVariable(SC));
if (getStorageClass() != SC)
- ClearLVCache();
+ ClearLinkageCache();
VarDeclBits.SClass = SC;
}
@@ -1714,9 +1720,6 @@
Body = B;
if (B)
EndRangeLoc = B->getLocEnd();
- for (redecl_iterator R = redecls_begin(), REnd = redecls_end(); R != REnd;
- ++R)
- R->ClearLVCache();
}
void FunctionDecl::setPure(bool P) {
@@ -1825,7 +1828,7 @@
void FunctionDecl::setStorageClass(StorageClass SC) {
assert(isLegalForFunction(SC));
if (getStorageClass() != SC)
- ClearLVCache();
+ ClearLinkageCache();
SClass = SC;
}
@@ -2595,8 +2598,8 @@
void TagDecl::setTypedefNameForAnonDecl(TypedefNameDecl *TDD) {
TypedefNameDeclOrQualifier = TDD;
if (TypeForDecl)
- const_cast<Type*>(TypeForDecl)->ClearLVCache();
- ClearLVCache();
+ const_cast<Type*>(TypeForDecl)->ClearLinkageCache();
+ ClearLinkageCache();
}
void TagDecl::startDefinition() {
Modified: cfe/trunk/lib/AST/Type.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=172305&r1=172304&r2=172305&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Sat Jan 12 00:42:30 2013
@@ -2190,7 +2190,7 @@
return std::make_pair(TypeBits.getLinkage(), TypeBits.getVisibility());
}
-void Type::ClearLVCache() {
+void Type::ClearLinkageCache() {
TypeBits.CacheValidAndVisibility = 0;
if (QualType(this, 0) != CanonicalType)
CanonicalType->TypeBits.CacheValidAndVisibility = 0;
Modified: cfe/trunk/lib/Sema/SemaAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAttr.cpp?rev=172305&r1=172304&r2=172305&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaAttr.cpp Sat Jan 12 00:42:30 2013
@@ -321,7 +321,6 @@
= (VisibilityAttr::VisibilityType) rawType;
SourceLocation loc = Stack->back().second;
- ND->ClearLVCache();
D->addAttr(::new (Context) VisibilityAttr(loc, Context, type));
}
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=172305&r1=172304&r2=172305&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Sat Jan 12 00:42:30 2013
@@ -1824,18 +1824,14 @@
bool Sema::mergeDeclAttribute(NamedDecl *D, InheritableAttr *Attr) {
InheritableAttr *NewAttr = NULL;
- if (AvailabilityAttr *AA = dyn_cast<AvailabilityAttr>(Attr)) {
+ if (AvailabilityAttr *AA = dyn_cast<AvailabilityAttr>(Attr))
NewAttr = mergeAvailabilityAttr(D, AA->getRange(), AA->getPlatform(),
AA->getIntroduced(), AA->getDeprecated(),
AA->getObsoleted(), AA->getUnavailable(),
AA->getMessage());
- if (NewAttr)
- D->ClearLVCache();
- } else if (VisibilityAttr *VA = dyn_cast<VisibilityAttr>(Attr)) {
+ else if (VisibilityAttr *VA = dyn_cast<VisibilityAttr>(Attr))
NewAttr = mergeVisibilityAttr(D, VA->getRange(), VA->getVisibility());
- if (NewAttr)
- D->ClearLVCache();
- } else if (DLLImportAttr *ImportA = dyn_cast<DLLImportAttr>(Attr))
+ else if (DLLImportAttr *ImportA = dyn_cast<DLLImportAttr>(Attr))
NewAttr = mergeDLLImportAttr(D, ImportA->getRange());
else if (DLLExportAttr *ExportA = dyn_cast<DLLExportAttr>(Attr))
NewAttr = mergeDLLExportAttr(D, ExportA->getRange());
@@ -6695,7 +6691,7 @@
}
VDecl->setTypeSourceInfo(DeducedType);
VDecl->setType(DeducedType->getType());
- VDecl->ClearLVCache();
+ VDecl->ClearLinkageCache();
// In ARC, infer lifetime.
if (getLangOpts().ObjCAutoRefCount && inferObjCARCLifetime(VDecl))
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=172305&r1=172304&r2=172305&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Sat Jan 12 00:42:30 2013
@@ -2043,7 +2043,6 @@
VersionTuple MergedDeprecated = Deprecated;
VersionTuple MergedObsoleted = Obsoleted;
bool FoundAny = false;
- bool DroppedAny = false;
if (D->hasAttrs()) {
AttrVec &Attrs = D->getAttrs();
@@ -2078,7 +2077,6 @@
Diag(OldAA->getLocation(), diag::warn_mismatched_availability);
Diag(Range.getBegin(), diag::note_previous_attribute);
Attrs.erase(Attrs.begin() + i);
- DroppedAny = true;
--e;
continue;
}
@@ -2098,7 +2096,6 @@
MergedIntroduced2, MergedDeprecated2,
MergedObsoleted2)) {
Attrs.erase(Attrs.begin() + i);
- DroppedAny = true;
--e;
continue;
}
@@ -2110,9 +2107,6 @@
}
}
- if (DroppedAny)
- D->ClearLVCache();
-
if (FoundAny &&
MergedIntroduced == Introduced &&
MergedDeprecated == Deprecated &&
@@ -2159,10 +2153,8 @@
Deprecated.Version,
Obsoleted.Version,
IsUnavailable, Str);
- if (NewAttr) {
+ if (NewAttr)
D->addAttr(NewAttr);
- ND->ClearLVCache();
- }
}
VisibilityAttr *Sema::mergeVisibilityAttr(Decl *D, SourceRange Range,
@@ -2179,8 +2171,6 @@
Diag(ExistingAttr->getLocation(), diag::err_mismatched_visibility);
Diag(Range.getBegin(), diag::note_previous_attribute);
D->dropAttr<VisibilityAttr>();
- NamedDecl *ND = cast<NamedDecl>(D);
- ND->ClearLVCache();
}
return ::new (Context) VisibilityAttr(Range, Context, Vis);
}
@@ -2224,11 +2214,8 @@
}
VisibilityAttr *NewAttr = S.mergeVisibilityAttr(D, Attr.getRange(), type);
- if (NewAttr) {
+ if (NewAttr)
D->addAttr(NewAttr);
- NamedDecl *ND = cast<NamedDecl>(D);
- ND->ClearLVCache();
- }
}
static void handleObjCMethodFamilyAttr(Sema &S, Decl *decl,
Modified: cfe/trunk/test/CodeGenCXX/visibility-inlines-hidden.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/visibility-inlines-hidden.cpp?rev=172305&r1=172304&r2=172305&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/visibility-inlines-hidden.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/visibility-inlines-hidden.cpp Sat Jan 12 00:42:30 2013
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown -fvisibility-inlines-hidden -emit-llvm -o - %s -O2 -disable-llvm-optzns | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -std=c++11 -fvisibility-inlines-hidden -emit-llvm -o - %s -O2 -disable-llvm-optzns | FileCheck %s
// The trickery with optimization in the run line is to get IR
// generation to emit available_externally function bodies, but not
@@ -147,3 +147,18 @@
template <int Idx_nocapture> void Op() {
}
}
+
+namespace test6 {
+ // just don't crash.
+ template <typename T>
+ void f(T x) {
+ }
+ struct C {
+ static void g() {
+ f([](){});
+ }
+ };
+ void g() {
+ C::g();
+ }
+}
Modified: cfe/trunk/test/CodeGenCXX/visibility.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/visibility.cpp?rev=172305&r1=172304&r2=172305&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/visibility.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/visibility.cpp Sat Jan 12 00:42:30 2013
@@ -1112,3 +1112,47 @@
// CHECK-HIDDEN: define linkonce_odr hidden void @_ZN6test604testINS_1bENS_1aEEEvv
}
}
+
+namespace test61 {
+ template <typename T1>
+ struct Class1
+ {
+ void f1() { f2(); }
+ inline void f2();
+ };
+ template<>
+ inline void Class1<int>::f2()
+ {
+ }
+ void g(Class1<int> *x) {
+ x->f1();
+ }
+}
+namespace test61 {
+ // Just test that we don't crash. Currently we apply this attribute. Current
+ // gcc issues a warning about it being unused since "the type is already
+ // defined". We should probably do the same.
+ template class __attribute__ ((visibility("hidden"))) Class1<int>;
+}
+
+namespace test62 {
+ template <typename T1>
+ struct Class1
+ {
+ void f1() { f2(); }
+ inline void f2() {}
+ };
+ template<>
+ inline void Class1<int>::f2()
+ {
+ }
+ void g(Class1<int> *x) {
+ x->f2();
+ }
+}
+namespace test62 {
+ template class __attribute__ ((visibility("hidden"))) Class1<int>;
+ // Just test that we don't crash. Currently we apply this attribute. Current
+ // gcc issues a warning about it being unused since "the type is already
+ // defined". We should probably do the same.
+}
More information about the cfe-commits
mailing list