From: Daniel Dunbar Date: Fri, 29 Oct 2010 15:19:36 +0000 (+0000) Subject: Revert r117644, "Apply visibility in IR gen to variables that are merely X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=034f55c1eb93aee61cef5a015eb1d5ba06d3b3d4;p=clang Revert r117644, "Apply visibility in IR gen to variables that are merely declared", it breaks things. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@117653 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 7c788ec56c..f7fd7b8d5a 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -140,25 +140,16 @@ static LVPair getLVForTemplateArgumentList(const TemplateArgumentList &TArgs) { 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()) return true; - if (const RecordDecl *Parent = dyn_cast(RD->getDeclContext())) - return HasExplicitVisibilityInHierarchy(Parent); - return false; -} - /// getLVForDecl - Get the cached linkage and visibility for the given /// declaration. /// -/// \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); +/// \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); static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D, - bool ConsiderGlobalVisibility) { + bool ConsiderGlobalSettings) { assert(D->getDeclContext()->getRedeclContext()->isFileContext() && "Not a name having namespace scope"); ASTContext &Context = D->getASTContext(); @@ -225,6 +216,10 @@ static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D, // 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 @@ -232,9 +227,6 @@ static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D, // // - an object or reference, unless it has internal linkage; or if (const VarDecl *Var = dyn_cast(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 @@ -281,9 +273,6 @@ static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D, 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 @@ -326,12 +315,11 @@ static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D, if (SpecInfo->getTemplateSpecializationKind() == TSK_ExplicitInstantiationDeclaration) - ConsiderGlobalVisibility = false; + ConsiderDashFVisibility = false; } - // -fvisibility only applies to function definitions. - if (ConsiderGlobalVisibility) - ConsiderGlobalVisibility = Function->hasBody(); + if (ConsiderDashFVisibility) + ConsiderDashFVisibility = Function->hasBody(); // - a named class (Clause 9), or an unnamed class defined in a // typedef declaration in which the class has the typedef name @@ -358,12 +346,12 @@ static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D, if (Spec->getTemplateSpecializationKind() == TSK_ExplicitInstantiationDeclaration) - ConsiderGlobalVisibility = false; + ConsiderDashFVisibility = false; } // Consider -fvisibility unless the type has C linkage. - if (ConsiderGlobalVisibility) - ConsiderGlobalVisibility = + if (ConsiderDashFVisibility) + ConsiderDashFVisibility = (Context.getLangOptions().CPlusPlus && !Tag->getDeclContext()->isExternCContext()); @@ -413,7 +401,7 @@ static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D, // If we have an explicit visibility attribute, merge that in. if (ExplicitVisibility) StandardV = GetVisibilityFromAttr(ExplicitVisibility); - else if (ConsiderGlobalVisibility) + else if (ConsiderDashFVisibility) StandardV = Context.getLangOptions().getVisibilityMode(); else StandardV = DefaultVisibility; // no-op @@ -425,7 +413,7 @@ static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D, } static LVPair getLVForClassMember(const NamedDecl *D, - bool ConsiderGlobalVisibility) { + bool ConsiderGlobalSettings) { // 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 @@ -437,9 +425,12 @@ static LVPair getLVForClassMember(const NamedDecl *D, (D->getDeclName() || cast(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. - // Always ignore global visibility settings during this. - LVPair ClassLV = getLVForDecl(cast(D->getDeclContext()), false); + LVPair ClassLV = getLVForDecl(cast(D->getDeclContext()), + ConsiderGlobalSettings && !VA); if (!isExternalLinkage(ClassLV.first)) return LVPair(NoLinkage, DefaultVisibility); @@ -449,21 +440,19 @@ static LVPair getLVForClassMember(const NamedDecl *D, // Start with the class's linkage and visibility. LVPair LV = ClassLV; - - // 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; + 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(D)) { + LVPair TypeLV = cast(D)->getType()->getLinkageAndVisibility(); + if (TypeLV.first != ExternalLinkage) + LV.first = minLinkage(LV.first, UniqueExternalLinkage); + LV.second = minVisibility(LV.second, TypeLV.second); } - bool HasExplicitVisibility = (VA || - HasExplicitVisibilityInHierarchy(cast(D->getDeclContext()))); - if (const CXXMethodDecl *MD = dyn_cast(D)) { - TemplateSpecializationKind TSK = TSK_Undeclared; - // If this is a method template specialization, use the linkage for // the template parameters and arguments. if (FunctionTemplateSpecializationInfo *Spec @@ -471,80 +460,24 @@ static LVPair getLVForClassMember(const NamedDecl *D, 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(); } - // 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) + // 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) LV.second = HiddenVisibility; - // Note that in contrast to basically every other situation, we - // *do* apply -fvisibility to method declarations. - - } else if (const CXXRecordDecl *RD = dyn_cast(D)) { - TemplateSpecializationKind TSK = TSK_Undeclared; - - if (const ClassTemplateSpecializationDecl *Spec - = dyn_cast(RD)) { - // Merge template argument/parameter information for member - // class template specializations. - LV = merge(LV, getLVForTemplateArgumentList(Spec->getTemplateArgs())); - LV = merge(LV, getLVForTemplateParameterList( + // Similarly for member class template specializations. + } else if (const ClassTemplateSpecializationDecl *Spec + = dyn_cast(D)) { + 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(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; diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp index 558a8a3cf8..efcdce70a0 100644 --- a/lib/CodeGen/CodeGenModule.cpp +++ b/lib/CodeGen/CodeGenModule.cpp @@ -164,17 +164,6 @@ void CodeGenModule::ErrorUnsupported(const Decl *D, const char *Type, 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. @@ -183,7 +172,15 @@ void CodeGenModule::setGlobalVisibility(llvm::GlobalValue *GV, return; } - GV->setVisibility(GetLLVMVisibility(D->getVisibility())); + 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!"); } /// Set the symbol visibility of type information (vtable and RTTI) @@ -937,18 +934,15 @@ CodeGenModule::GetOrCreateLLVMGlobal(llvm::StringRef MangledName, // handling. GV->setConstant(DeclIsConstantGlobal(Context, D)); - // Set linkage and visibility in case we never see a definition. - std::pair LV = D->getLinkageAndVisibility(); - if (LV.first != ExternalLinkage) { - GV->setLinkage(llvm::GlobalValue::InternalLinkage); - } else { - if (D->hasAttr()) - GV->setLinkage(llvm::GlobalValue::DLLImportLinkage); - else if (D->hasAttr() || D->hasAttr()) - GV->setLinkage(llvm::GlobalValue::ExternalWeakLinkage); + // FIXME: Merge with other attribute handling code. + if (D->getStorageClass() == SC_PrivateExtern) + GV->setVisibility(llvm::GlobalValue::HiddenVisibility); - GV->setVisibility(GetLLVMVisibility(LV.second)); - } + if (D->hasAttr()) + GV->setLinkage(llvm::GlobalValue::DLLImportLinkage); + else if (D->hasAttr() || + D->hasAttr()) + GV->setLinkage(llvm::GlobalValue::ExternalWeakLinkage); GV->setThreadLocal(D->isThreadSpecified()); } diff --git a/test/CodeGenCXX/visibility.cpp b/test/CodeGenCXX/visibility.cpp index 73182ac253..5a6ded88b1 100644 --- a/test/CodeGenCXX/visibility.cpp +++ b/test/CodeGenCXX/visibility.cpp @@ -10,14 +10,6 @@ // 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 { @@ -173,37 +165,3 @@ namespace Test11 { // 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; -};