[cfe-commits] r117644 - in /cfe/trunk: lib/AST/Decl.cpp lib/CodeGen/CodeGenModule.cpp test/CodeGenCXX/visibility.cpp
John McCall
rjmccall at apple.com
Fri Oct 29 00:49:42 PDT 2010
Author: rjmccall
Date: Fri Oct 29 02:49:41 2010
New Revision: 117644
URL: http://llvm.org/viewvc/llvm-project?rev=117644&view=rev
Log:
Apply visibility in IR gen to variables that are merely declared
and never defined. We were previously emitting these with default
visibility unless they were declared with private_extern.
Ignore global visibility settings when computing visibility for
a declaration's context, and key several conditions on whether a
visibility attribute exists anywhere in the hierarchy as opposed
to whether it exists at the current level.
Modified:
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGenCXX/visibility.cpp
Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=117644&r1=117643&r2=117644&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Fri Oct 29 02:49:41 2010
@@ -140,16 +140,25 @@
TArgs.flat_size());
}
+/// Answers whether the given class, or any containing class, has an
+/// explicit visibility attribute.
+static bool HasExplicitVisibilityInHierarchy(const RecordDecl *RD) {
+ if (RD->hasAttr<VisibilityAttr>()) return true;
+ if (const RecordDecl *Parent = dyn_cast<RecordDecl>(RD->getDeclContext()))
+ return HasExplicitVisibilityInHierarchy(Parent);
+ return false;
+}
+
/// getLVForDecl - Get the cached linkage and visibility for the given
/// declaration.
///
-/// \param ConsiderGlobalSettings - Whether to honor global visibility
-/// settings. This is false when computing the visibility of the
-/// context of a declaration with an explicit visibility attribute.
-static LVPair getLVForDecl(const NamedDecl *D, bool ConsiderGlobalSettings);
+/// \param ConsiderGlobalVisibility - Whether to honor global visibility
+/// settings. This is generally false when computing the visibility
+/// of the context of a declaration.
+static LVPair getLVForDecl(const NamedDecl *D, bool ConsiderGlobalVisibility);
static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D,
- bool ConsiderGlobalSettings) {
+ bool ConsiderGlobalVisibility) {
assert(D->getDeclContext()->getRedeclContext()->isFileContext() &&
"Not a name having namespace scope");
ASTContext &Context = D->getASTContext();
@@ -216,10 +225,6 @@
// external.
LVPair LV(ExternalLinkage, DefaultVisibility);
- // We ignore -fvisibility on non-definitions and explicit
- // instantiation declarations.
- bool ConsiderDashFVisibility = ConsiderGlobalSettings;
-
// C++ [basic.link]p4:
// A name having namespace scope has external linkage if it is the
@@ -227,6 +232,9 @@
//
// - an object or reference, unless it has internal linkage; or
if (const VarDecl *Var = dyn_cast<VarDecl>(D)) {
+ // GCC applies the following optimization to variables and static
+ // data members, but not to functions:
+ //
// Modify the variable's LV by the LV of its type unless this is
// C or extern "C". This follows from [basic.link]p9:
// A type without linkage shall not be used as the type of a
@@ -273,6 +281,9 @@
if (PrevLV.first) LV.first = PrevLV.first;
LV.second = minVisibility(LV.second, PrevLV.second);
}
+
+ // Don't consider -fvisibility for extern declarations.
+ ConsiderGlobalVisibility = false;
}
// - a function, unless it has internal linkage; or
@@ -315,11 +326,12 @@
if (SpecInfo->getTemplateSpecializationKind()
== TSK_ExplicitInstantiationDeclaration)
- ConsiderDashFVisibility = false;
+ ConsiderGlobalVisibility = false;
}
- if (ConsiderDashFVisibility)
- ConsiderDashFVisibility = Function->hasBody();
+ // -fvisibility only applies to function definitions.
+ if (ConsiderGlobalVisibility)
+ ConsiderGlobalVisibility = Function->hasBody();
// - a named class (Clause 9), or an unnamed class defined in a
// typedef declaration in which the class has the typedef name
@@ -346,12 +358,12 @@
if (Spec->getTemplateSpecializationKind()
== TSK_ExplicitInstantiationDeclaration)
- ConsiderDashFVisibility = false;
+ ConsiderGlobalVisibility = false;
}
// Consider -fvisibility unless the type has C linkage.
- if (ConsiderDashFVisibility)
- ConsiderDashFVisibility =
+ if (ConsiderGlobalVisibility)
+ ConsiderGlobalVisibility =
(Context.getLangOptions().CPlusPlus &&
!Tag->getDeclContext()->isExternCContext());
@@ -401,7 +413,7 @@
// If we have an explicit visibility attribute, merge that in.
if (ExplicitVisibility)
StandardV = GetVisibilityFromAttr(ExplicitVisibility);
- else if (ConsiderDashFVisibility)
+ else if (ConsiderGlobalVisibility)
StandardV = Context.getLangOptions().getVisibilityMode();
else
StandardV = DefaultVisibility; // no-op
@@ -413,7 +425,7 @@
}
static LVPair getLVForClassMember(const NamedDecl *D,
- bool ConsiderGlobalSettings) {
+ bool ConsiderGlobalVisibility) {
// Only certain class members have linkage. Note that fields don't
// really have linkage, but it's convenient to say they do for the
// purposes of calculating linkage of pointer-to-data-member
@@ -425,12 +437,9 @@
(D->getDeclName() || cast<TagDecl>(D)->getTypedefForAnonDecl()))))
return LVPair(NoLinkage, DefaultVisibility);
- // If we have an explicit visibility attribute, merge that in.
- const VisibilityAttr *VA = GetExplicitVisibility(D);
-
// Class members only have linkage if their class has external linkage.
- LVPair ClassLV = getLVForDecl(cast<RecordDecl>(D->getDeclContext()),
- ConsiderGlobalSettings && !VA);
+ // Always ignore global visibility settings during this.
+ LVPair ClassLV = getLVForDecl(cast<RecordDecl>(D->getDeclContext()), false);
if (!isExternalLinkage(ClassLV.first))
return LVPair(NoLinkage, DefaultVisibility);
@@ -440,19 +449,21 @@
// Start with the class's linkage and visibility.
LVPair LV = ClassLV;
- if (VA) LV.second = minVisibility(LV.second, GetVisibilityFromAttr(VA));
- // If it's a variable declaration and we don't have an explicit
- // visibility attribute, apply the LV from its type.
- // See the comment about namespace-scope variable decls above.
- if (!VA && isa<VarDecl>(D)) {
- LVPair TypeLV = cast<VarDecl>(D)->getType()->getLinkageAndVisibility();
- if (TypeLV.first != ExternalLinkage)
- LV.first = minLinkage(LV.first, UniqueExternalLinkage);
- LV.second = minVisibility(LV.second, TypeLV.second);
+ // If we have an explicit visibility attribute, merge that in and
+ // ignore global visibility settings.
+ const VisibilityAttr *VA = GetExplicitVisibility(D);
+ if (VA) {
+ LV.second = minVisibility(LV.second, GetVisibilityFromAttr(VA));
+ ConsiderGlobalVisibility = false;
}
+ bool HasExplicitVisibility = (VA ||
+ HasExplicitVisibilityInHierarchy(cast<RecordDecl>(D->getDeclContext())));
+
if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D)) {
+ TemplateSpecializationKind TSK = TSK_Undeclared;
+
// If this is a method template specialization, use the linkage for
// the template parameters and arguments.
if (FunctionTemplateSpecializationInfo *Spec
@@ -460,24 +471,80 @@
LV = merge(LV, getLVForTemplateArgumentList(*Spec->TemplateArguments));
LV = merge(LV, getLVForTemplateParameterList(
Spec->getTemplate()->getTemplateParameters()));
+
+ TSK = Spec->getTemplateSpecializationKind();
+ } else if (MemberSpecializationInfo *MSI =
+ MD->getMemberSpecializationInfo()) {
+ TSK = MSI->getTemplateSpecializationKind();
}
- // If -fvisibility-inlines-hidden was provided, then inline C++
- // member functions get "hidden" visibility if they don't have an
- // explicit visibility attribute.
- if (ConsiderGlobalSettings && !VA && MD->isInlined() &&
- LV.second > HiddenVisibility &&
- D->getASTContext().getLangOptions().InlineVisibilityHidden &&
- MD->getTemplateSpecializationKind()
- != TSK_ExplicitInstantiationDeclaration)
+ // Ignore global visibility if it's an extern template.
+ if (ConsiderGlobalVisibility)
+ ConsiderGlobalVisibility = (TSK != TSK_ExplicitInstantiationDeclaration);
+
+ // If we're paying attention to global visibility, apply
+ // -finline-visibility-hidden if this is an inline method.
+ //
+ // Note that we ignore the existence of visibility attributes
+ // on containing classes when deciding whether to do this.
+ if (ConsiderGlobalVisibility && MD->isInlined() &&
+ MD->getASTContext().getLangOptions().InlineVisibilityHidden)
LV.second = HiddenVisibility;
- // Similarly for member class template specializations.
- } else if (const ClassTemplateSpecializationDecl *Spec
- = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
- LV = merge(LV, getLVForTemplateArgumentList(Spec->getTemplateArgs()));
- LV = merge(LV, getLVForTemplateParameterList(
+ // Note that in contrast to basically every other situation, we
+ // *do* apply -fvisibility to method declarations.
+
+ } else if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(D)) {
+ TemplateSpecializationKind TSK = TSK_Undeclared;
+
+ if (const ClassTemplateSpecializationDecl *Spec
+ = dyn_cast<ClassTemplateSpecializationDecl>(RD)) {
+ // Merge template argument/parameter information for member
+ // class template specializations.
+ LV = merge(LV, getLVForTemplateArgumentList(Spec->getTemplateArgs()));
+ LV = merge(LV, getLVForTemplateParameterList(
Spec->getSpecializedTemplate()->getTemplateParameters()));
+
+ TSK = Spec->getTemplateSpecializationKind();
+ } else if (MemberSpecializationInfo *MSI =
+ RD->getMemberSpecializationInfo()) {
+ TSK = MSI->getTemplateSpecializationKind();
+ }
+
+ // Ignore global visibility if it's an extern template.
+ if (ConsiderGlobalVisibility)
+ ConsiderGlobalVisibility = (TSK != TSK_ExplicitInstantiationDeclaration);
+
+ // Static data members.
+ } else if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
+ // If we don't have explicit visibility information in the
+ // hierarchy, apply the LV from its type. See the comment about
+ // namespace-scope variables for justification for this
+ // optimization.
+ if (!HasExplicitVisibility) {
+ LVPair TypeLV = VD->getType()->getLinkageAndVisibility();
+ if (TypeLV.first != ExternalLinkage)
+ LV.first = minLinkage(LV.first, UniqueExternalLinkage);
+ LV.second = minVisibility(LV.second, TypeLV.second);
+ }
+
+ // Ignore global visibility if it's an extern template or
+ // just a declaration.
+ if (ConsiderGlobalVisibility)
+ ConsiderGlobalVisibility =
+ (VD->getDefinition() &&
+ VD->getTemplateSpecializationKind()
+ != TSK_ExplicitInstantiationDeclaration);
+ }
+
+ // Suppress -fvisibility if we have explicit visibility on any of
+ // our ancestors.
+ ConsiderGlobalVisibility &= !HasExplicitVisibility;
+
+ // Apply -fvisibility if desired.
+ if (ConsiderGlobalVisibility && LV.second != HiddenVisibility) {
+ LV.second = minVisibility(LV.second,
+ D->getASTContext().getLangOptions().getVisibilityMode());
}
return LV;
Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=117644&r1=117643&r2=117644&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Fri Oct 29 02:49:41 2010
@@ -164,6 +164,17 @@
getDiags().Report(Context.getFullLoc(D->getLocation()), DiagID) << Msg;
}
+static llvm::GlobalValue::VisibilityTypes GetLLVMVisibility(Visibility V) {
+ switch (V) {
+ case DefaultVisibility: return llvm::GlobalValue::DefaultVisibility;
+ case HiddenVisibility: return llvm::GlobalValue::HiddenVisibility;
+ case ProtectedVisibility: return llvm::GlobalValue::ProtectedVisibility;
+ }
+ llvm_unreachable("unknown visibility!");
+ return llvm::GlobalValue::DefaultVisibility;
+}
+
+
void CodeGenModule::setGlobalVisibility(llvm::GlobalValue *GV,
const NamedDecl *D) const {
// Internal definitions always have default visibility.
@@ -172,15 +183,7 @@
return;
}
- switch (D->getVisibility()) {
- case DefaultVisibility:
- return GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
- case HiddenVisibility:
- return GV->setVisibility(llvm::GlobalValue::HiddenVisibility);
- case ProtectedVisibility:
- return GV->setVisibility(llvm::GlobalValue::ProtectedVisibility);
- }
- llvm_unreachable("unknown visibility!");
+ GV->setVisibility(GetLLVMVisibility(D->getVisibility()));
}
/// Set the symbol visibility of type information (vtable and RTTI)
@@ -934,15 +937,18 @@
// handling.
GV->setConstant(DeclIsConstantGlobal(Context, D));
- // FIXME: Merge with other attribute handling code.
- if (D->getStorageClass() == SC_PrivateExtern)
- GV->setVisibility(llvm::GlobalValue::HiddenVisibility);
-
- if (D->hasAttr<DLLImportAttr>())
- GV->setLinkage(llvm::GlobalValue::DLLImportLinkage);
- else if (D->hasAttr<WeakAttr>() ||
- D->hasAttr<WeakImportAttr>())
- GV->setLinkage(llvm::GlobalValue::ExternalWeakLinkage);
+ // Set linkage and visibility in case we never see a definition.
+ std::pair<Linkage,Visibility> LV = D->getLinkageAndVisibility();
+ if (LV.first != ExternalLinkage) {
+ GV->setLinkage(llvm::GlobalValue::InternalLinkage);
+ } else {
+ if (D->hasAttr<DLLImportAttr>())
+ GV->setLinkage(llvm::GlobalValue::DLLImportLinkage);
+ else if (D->hasAttr<WeakAttr>() || D->hasAttr<WeakImportAttr>())
+ GV->setLinkage(llvm::GlobalValue::ExternalWeakLinkage);
+
+ GV->setVisibility(GetLLVMVisibility(LV.second));
+ }
GV->setThreadLocal(D->isThreadSpecified());
}
Modified: cfe/trunk/test/CodeGenCXX/visibility.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/visibility.cpp?rev=117644&r1=117643&r2=117644&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/visibility.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/visibility.cpp Fri Oct 29 02:49:41 2010
@@ -10,6 +10,14 @@
// CHECK: @_ZN5Test71bE = global
// CHECK: @test9_var = global
// CHECK-HIDDEN: @test9_var = global
+// CHECK: @_ZN6Test121A6hiddenE = external hidden global
+// CHECK: @_ZN6Test121A7visibleE = external global
+// CHECK-HIDDEN: @_ZN6Test121A6hiddenE = external hidden global
+// CHECK-HIDDEN: @_ZN6Test121A7visibleE = external global
+// CHECK: @_ZN6Test131B1aE = hidden global
+// CHECK: @_ZN6Test131C1aE = global
+// CHECK-HIDDEN: @_ZN6Test131B1aE = hidden global
+// CHECK-HIDDEN: @_ZN6Test131C1aE = global
// CHECK: @_ZTVN5Test63fooE = weak_odr hidden constant
namespace Test1 {
@@ -165,3 +173,37 @@
// CHECK-HIDDEN: define linkonce_odr hidden void @_ZN6Test111A3fooEv(
// CHECK-HIDDEN: define linkonce_odr void @_ZN6Test111A3barEv(
}
+
+// Tested at top of file.
+namespace Test12 {
+ struct A {
+ // This is hidden in all cases: the explicit attribute takes
+ // priority over -fvisibility on the parent.
+ static int hidden HIDDEN;
+
+ // This is default in all cases because it's only a declaration.
+ static int visible;
+ };
+
+ void test() {
+ A::hidden = 0;
+ A::visible = 0;
+ }
+}
+
+// Tested at top of file.
+namespace Test13 {
+ struct HIDDEN A {};
+
+ // Should be hidden in all cases.
+ struct B {
+ static A a;
+ };
+ A B::a;
+
+ // Should be default in all cases.
+ struct DEFAULT C {
+ static A a;
+ };
+ A C::a;
+};
More information about the cfe-commits
mailing list