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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@117644 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index f7fd7b8d5a9e708b1a0f9135d79e8c529f50b64d..7c788ec56c9388f5bfbf0af89d3dc0ee85dcf606 100644 (file)
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -140,16 +140,25 @@ 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<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 @@ 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
@@ -227,6 +232,9 @@ static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D,
   //
   //     - 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 @@ 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
@@ -315,11 +326,12 @@ static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D,
 
       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 @@ static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D,
 
       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 @@ static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D,
     // 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 getLVForNamespaceScopeDecl(const NamedDecl *D,
 }
 
 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 @@ static LVPair getLVForClassMember(const NamedDecl *D,
          (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 @@ static LVPair getLVForClassMember(const NamedDecl *D,
 
   // 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 @@ 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();
     }
 
-    // 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;

diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp
index efcdce70a05f0164c1f936dca59f1c7a3e00422d..558a8a3cf8b315912600992f5b877172525bd6d2 100644 (file)
--- a/lib/CodeGen/CodeGenModule.cpp
+++ b/lib/CodeGen/CodeGenModule.cpp
@@ -164,6 +164,17 @@ 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.
@@ -172,15 +183,7 @@ void CodeGenModule::setGlobalVisibility(llvm::GlobalValue *GV,
     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 @@ CodeGenModule::GetOrCreateLLVMGlobal(llvm::StringRef MangledName,
     // handling.
     GV->setConstant(DeclIsConstantGlobal(Context, D));
 
-    // FIXME: Merge with other attribute handling code.
-    if (D->getStorageClass() == SC_PrivateExtern)
-      GV->setVisibility(llvm::GlobalValue::HiddenVisibility);
+    // 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);
 
-    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());
   }

diff --git a/test/CodeGenCXX/visibility.cpp b/test/CodeGenCXX/visibility.cpp
index 5a6ded88b1d93460c4d88b140a26ba1031838b0b..73182ac253ca573c426754fc241421f2d2639d1b 100644 (file)
--- a/test/CodeGenCXX/visibility.cpp
+++ b/test/CodeGenCXX/visibility.cpp
@@ -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 @@ 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;
+};