[cfe-commits] r117729 - in /cfe/trunk: include/clang/AST/Decl.h lib/AST/Decl.cpp lib/CodeGen/CodeGenModule.cpp test/CodeGenCXX/visibility.cpp
John McCall
rjmccall at apple.com
Fri Oct 29 15:22:43 PDT 2010
Author: rjmccall
Date: Fri Oct 29 17:22:43 2010
New Revision: 117729
URL: http://llvm.org/viewvc/llvm-project?rev=117729&view=rev
Log:
Restore r117644, this time properly ignoring -fvisibility and type visibility
for namespace-scope variable declarations.
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/include/clang/AST/Decl.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.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=117729&r1=117728&r2=117729&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Fri Oct 29 17:22:43 2010
@@ -694,6 +694,10 @@
/// definition.
DefinitionKind isThisDeclarationADefinition() const;
+ /// \brief Check whether this variable is defined in this
+ /// translation unit.
+ DefinitionKind hasDefinition() const;
+
/// \brief Get the tentative definition that acts as the real definition in
/// a TU. Returns null if there is a proper definition available.
VarDecl *getActingDefinition();
Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=117729&r1=117728&r2=117729&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Fri Oct 29 17:22:43 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,11 @@
//
// - an object or reference, unless it has internal linkage; or
if (const VarDecl *Var = dyn_cast<VarDecl>(D)) {
+ bool isDeclaration = (Var->hasDefinition() == VarDecl::DeclarationOnly);
+
+ // 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
@@ -245,12 +255,20 @@
//
// Note that we don't want to make the variable non-external
// because of this, but unique-external linkage suits us.
- if (Context.getLangOptions().CPlusPlus && !Var->isExternC() &&
- !ExplicitVisibility) {
+ if (Context.getLangOptions().CPlusPlus && !ExplicitVisibility &&
+ !Var->isExternC()) {
LVPair TypeLV = Var->getType()->getLinkageAndVisibility();
if (TypeLV.first != ExternalLinkage)
return LVPair(UniqueExternalLinkage, DefaultVisibility);
- LV.second = minVisibility(LV.second, TypeLV.second);
+
+ // Otherwise, ignore type visibility for declarations.
+ if (!isDeclaration)
+ LV.second = minVisibility(LV.second, TypeLV.second);
+ }
+
+ // Don't consider -fvisibility for pure declarations.
+ if (isDeclaration) {
+ ConsiderGlobalVisibility = false;
}
if (!Context.getLangOptions().CPlusPlus &&
@@ -315,11 +333,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 +365,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 +420,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 +432,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 +444,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 +456,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 +478,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;
@@ -962,6 +1036,17 @@
return 0;
}
+VarDecl::DefinitionKind VarDecl::hasDefinition() const {
+ DefinitionKind Kind = DeclarationOnly;
+
+ const VarDecl *First = getFirstDeclaration();
+ for (redecl_iterator I = First->redecls_begin(), E = First->redecls_end();
+ I != E; ++I)
+ Kind = std::max(Kind, (*I)->isThisDeclarationADefinition());
+
+ return Kind;
+}
+
const Expr *VarDecl::getAnyInitializer(const VarDecl *&D) const {
redecl_iterator I = redecls_begin(), E = redecls_end();
while (I != E && !I->getInit())
Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=117729&r1=117728&r2=117729&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Fri Oct 29 17:22:43 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=117729&r1=117728&r2=117729&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/visibility.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/visibility.cpp Fri Oct 29 17:22:43 2010
@@ -10,6 +10,16 @@
// 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: @_ZN6Test143varE = external global
+// CHECK-HIDDEN: @_ZN6Test143varE = external global
// CHECK: @_ZTVN5Test63fooE = weak_odr hidden constant
namespace Test1 {
@@ -165,3 +175,46 @@
// 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;
+};
+
+// Tested at top of file.
+namespace Test14 {
+ // Neither the visibility of the type nor -fvisibility=hidden should
+ // apply to declarations.
+ extern struct A *var;
+
+ struct A *test() { return var; }
+}
More information about the cfe-commits
mailing list