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

1 files changed, 112 insertions, 45 deletions

diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index f7fd7b8d5a..7c788ec56c 100644
--- 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;