[cfe-commits] r117781 - in /cfe/trunk: include/clang/AST/Decl.h lib/AST/Decl.cpp lib/AST/Type.cpp lib/CodeGen/CGRTTI.cpp lib/CodeGen/CGVTT.cpp lib/CodeGen/CGVTables.cpp lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h test/CodeGenCXX/visibility.cpp

John McCall rjmccall at apple.com
Sat Oct 30 04:50:41 PDT 2010


Author: rjmccall
Date: Sat Oct 30 06:50:40 2010
New Revision: 117781

URL: http://llvm.org/viewvc/llvm-project?rev=117781&view=rev
Log:
Better solution:  calculate the visibility of functions and variables
independently of whether they're definitions, then teach IR generation to
ignore non-explicit visibility when emitting declarations.  Use this to
make sure that RTTI, vtables, and VTTs get the right visibility.

More of rdar://problem/8613093


Modified:
    cfe/trunk/include/clang/AST/Decl.h
    cfe/trunk/lib/AST/Decl.cpp
    cfe/trunk/lib/AST/Type.cpp
    cfe/trunk/lib/CodeGen/CGRTTI.cpp
    cfe/trunk/lib/CodeGen/CGVTT.cpp
    cfe/trunk/lib/CodeGen/CGVTables.cpp
    cfe/trunk/lib/CodeGen/CodeGenModule.cpp
    cfe/trunk/lib/CodeGen/CodeGenModule.h
    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=117781&r1=117780&r2=117781&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Sat Oct 30 06:50:40 2010
@@ -196,14 +196,81 @@
   /// determine whether it's an instance member of its class.
   bool isCXXInstanceMember() const;
 
+  class LinkageInfo {
+    Linkage linkage_;
+    Visibility visibility_;
+    bool explicit_;
+
+  public:
+    LinkageInfo() : linkage_(ExternalLinkage), visibility_(DefaultVisibility),
+                    explicit_(false) {}
+    LinkageInfo(Linkage L, Visibility V, bool E)
+      : linkage_(L), visibility_(V), explicit_(E) {}
+
+    static LinkageInfo external() {
+      return LinkageInfo();
+    }
+    static LinkageInfo internal() {
+      return LinkageInfo(InternalLinkage, DefaultVisibility, false);
+    }
+    static LinkageInfo uniqueExternal() {
+      return LinkageInfo(UniqueExternalLinkage, DefaultVisibility, false);
+    }
+    static LinkageInfo none() {
+      return LinkageInfo(NoLinkage, DefaultVisibility, false);
+    }
+
+    Linkage linkage() const { return linkage_; }
+    Visibility visibility() const { return visibility_; }
+    bool visibilityExplicit() const { return explicit_; }
+
+    void setLinkage(Linkage L) { linkage_ = L; }
+    void setVisibility(Visibility V) { visibility_ = V; }
+    void setVisibility(Visibility V, bool E) { visibility_ = V; explicit_ = E; }
+    void setVisibility(LinkageInfo Other) {
+      setVisibility(Other.visibility(), Other.visibilityExplicit());
+    }
+
+    void mergeLinkage(Linkage L) {
+      setLinkage(minLinkage(linkage(), L));
+    }
+    void mergeLinkage(LinkageInfo Other) {
+      setLinkage(minLinkage(linkage(), Other.linkage()));
+    }
+
+    void mergeVisibility(Visibility V) {
+      setVisibility(minVisibility(visibility(), V));
+    }
+    void mergeVisibility(Visibility V, bool E) {
+      setVisibility(minVisibility(visibility(), V), visibilityExplicit() || E);
+    }
+    void mergeVisibility(LinkageInfo Other) {
+      mergeVisibility(Other.visibility(), Other.visibilityExplicit());
+    }
+
+    void merge(LinkageInfo Other) {
+      mergeLinkage(Other);
+      mergeVisibility(Other);
+    }
+    void merge(std::pair<Linkage,Visibility> LV) {
+      mergeLinkage(LV.first);
+      mergeVisibility(LV.second);
+    }
+
+    friend LinkageInfo merge(LinkageInfo L, LinkageInfo R) {
+      L.merge(R);
+      return L;
+    }
+  };
+
   /// \brief Determine what kind of linkage this entity has.
-  Linkage getLinkage() const { return getLinkageAndVisibility().first; }
+  Linkage getLinkage() const { return getLinkageAndVisibility().linkage(); }
 
   /// \brief Determines the visibility of this entity.
-  Visibility getVisibility() const { return getLinkageAndVisibility().second; }
+  Visibility getVisibility() const { return getLinkageAndVisibility().visibility(); }
 
   /// \brief Determines the linkage and visibility of this entity.
-  std::pair<Linkage,Visibility> getLinkageAndVisibility() const;
+  LinkageInfo getLinkageAndVisibility() const;
 
   /// \brief Looks through UsingDecls and ObjCCompatibleAliasDecls for
   /// the underlying named decl.

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=117781&r1=117780&r2=117781&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Sat Oct 30 06:50:40 2010
@@ -62,12 +62,19 @@
   return DefaultVisibility;
 }
 
+typedef NamedDecl::LinkageInfo LinkageInfo;
 typedef std::pair<Linkage,Visibility> LVPair;
+
 static LVPair merge(LVPair L, LVPair R) {
   return LVPair(minLinkage(L.first, R.first),
                 minVisibility(L.second, R.second));
 }
 
+static LVPair merge(LVPair L, LinkageInfo R) {
+  return LVPair(minLinkage(L.first, R.linkage()),
+                minVisibility(L.second, R.visibility()));
+}
+
 /// \brief Get the most restrictive linkage for the types in the given
 /// template parameter list.
 static LVPair 
@@ -84,8 +91,7 @@
 
     if (TemplateTemplateParmDecl *TTP
                                    = dyn_cast<TemplateTemplateParmDecl>(*P)) {
-      LV =
-        merge(LV, getLVForTemplateParameterList(TTP->getTemplateParameters()));
+      LV = merge(LV, getLVForTemplateParameterList(TTP->getTemplateParameters()));
     }
   }
 
@@ -116,7 +122,7 @@
         if (NamedDecl *ND = dyn_cast<NamedDecl>(D))
           LV = merge(LV, ND->getLinkageAndVisibility());
         if (ValueDecl *VD = dyn_cast<ValueDecl>(D))
-          LV = merge(LV, VD->getType()->getLinkageAndVisibility());
+          LV = merge(LV, VD->getLinkageAndVisibility());
       }
       break;
 
@@ -135,30 +141,23 @@
   return LV;
 }
 
-static LVPair getLVForTemplateArgumentList(const TemplateArgumentList &TArgs) {
+static LVPair
+getLVForTemplateArgumentList(const TemplateArgumentList &TArgs) {
   return getLVForTemplateArgumentList(TArgs.getFlatArgumentList(), 
                                       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 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 LinkageInfo getLVForDecl(const NamedDecl *D,
+                                bool ConsiderGlobalVisibility);
 
-static LVPair getLVForNamespaceScopeDecl(const NamedDecl *D,
-                                         bool ConsiderGlobalVisibility) {
+static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D,
+                                              bool ConsiderGlobalVisibility) {
   assert(D->getDeclContext()->getRedeclContext()->isFileContext() &&
          "Not a name having namespace scope");
   ASTContext &Context = D->getASTContext();
@@ -172,7 +171,7 @@
   if (const VarDecl *Var = dyn_cast<VarDecl>(D)) {
     // Explicitly declared static.
     if (Var->getStorageClass() == SC_Static)
-      return LVPair(InternalLinkage, DefaultVisibility);
+      return LinkageInfo::internal();
 
     // - an object or reference that is explicitly declared const
     //   and neither explicitly declared extern nor previously
@@ -190,7 +189,7 @@
           FoundExtern = true;
       
       if (!FoundExtern)
-        return LVPair(InternalLinkage, DefaultVisibility);
+        return LinkageInfo::internal();
     }
   } else if (isa<FunctionDecl>(D) || isa<FunctionTemplateDecl>(D)) {
     // C++ [temp]p4:
@@ -205,17 +204,15 @@
 
     // Explicitly declared static.
     if (Function->getStorageClass() == SC_Static)
-      return LVPair(InternalLinkage, DefaultVisibility);
+      return LinkageInfo(InternalLinkage, DefaultVisibility, false);
   } else if (const FieldDecl *Field = dyn_cast<FieldDecl>(D)) {
     //   - a data member of an anonymous union.
     if (cast<RecordDecl>(Field->getDeclContext())->isAnonymousStructOrUnion())
-      return LVPair(InternalLinkage, DefaultVisibility);
+      return LinkageInfo::internal();
   }
 
   if (D->isInAnonymousNamespace())
-    return LVPair(UniqueExternalLinkage, DefaultVisibility);
-
-  const VisibilityAttr *ExplicitVisibility = GetExplicitVisibility(D);
+    return LinkageInfo::uniqueExternal();
 
   // Set up the defaults.
 
@@ -223,7 +220,11 @@
   //   If the declaration of an identifier for an object has file
   //   scope and no storage-class specifier, its linkage is
   //   external.
-  LVPair LV(ExternalLinkage, DefaultVisibility);
+  LinkageInfo LV;
+
+  if (const VisibilityAttr *VA = GetExplicitVisibility(D)) {
+    LV.setVisibility(GetVisibilityFromAttr(VA), true);
+  }
 
   // C++ [basic.link]p4:
 
@@ -232,8 +233,6 @@
   //
   //     - 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:
     //
@@ -258,20 +257,16 @@
     if (Context.getLangOptions().CPlusPlus && !Var->isExternC()) {
       LVPair TypeLV = Var->getType()->getLinkageAndVisibility();
       if (TypeLV.first != ExternalLinkage)
-        return LVPair(UniqueExternalLinkage, DefaultVisibility);
-      if (!isDeclaration && !ExplicitVisibility)
-        LV.second = minVisibility(LV.second, TypeLV.second);
+        return LinkageInfo::uniqueExternal();
+      if (!LV.visibilityExplicit())
+        LV.mergeVisibility(TypeLV.second);
     }
 
-    // Don't consider -fvisibility for pure declarations.
-    if (isDeclaration)
-      ConsiderGlobalVisibility = false;
-
     if (!Context.getLangOptions().CPlusPlus &&
         (Var->getStorageClass() == SC_Extern ||
          Var->getStorageClass() == SC_PrivateExtern)) {
       if (Var->getStorageClass() == SC_PrivateExtern)
-        LV.second = HiddenVisibility;
+        LV.setVisibility(HiddenVisibility, true);
 
       // C99 6.2.2p4:
       //   For an identifier declared with the storage-class specifier
@@ -283,9 +278,9 @@
       //   is visible, or if the prior declaration specifies no
       //   linkage, then the identifier has external linkage.
       if (const VarDecl *PrevVar = Var->getPreviousDeclaration()) {
-        LVPair PrevLV = PrevVar->getLinkageAndVisibility();
-        if (PrevLV.first) LV.first = PrevLV.first;
-        LV.second = minVisibility(LV.second, PrevLV.second);
+        LinkageInfo PrevLV = PrevVar->getLinkageAndVisibility();
+        if (PrevLV.linkage()) LV.setLinkage(PrevLV.linkage());
+        LV.mergeVisibility(PrevLV);
       }
     }
 
@@ -315,27 +310,19 @@
       //   is visible, or if the prior declaration specifies no
       //   linkage, then the identifier has external linkage.
       if (const FunctionDecl *PrevFunc = Function->getPreviousDeclaration()) {
-        LVPair PrevLV = PrevFunc->getLinkageAndVisibility();
-        if (PrevLV.first) LV.first = PrevLV.first;
-        LV.second = minVisibility(LV.second, PrevLV.second);
+        LinkageInfo PrevLV = PrevFunc->getLinkageAndVisibility();
+        if (PrevLV.linkage()) LV.setLinkage(PrevLV.linkage());
+        LV.mergeVisibility(PrevLV);
       }
     }
 
     if (FunctionTemplateSpecializationInfo *SpecInfo
                                = Function->getTemplateSpecializationInfo()) {
-      LV = merge(LV, SpecInfo->getTemplate()->getLinkageAndVisibility());
+      LV.merge(SpecInfo->getTemplate()->getLinkageAndVisibility());
       const TemplateArgumentList &TemplateArgs = *SpecInfo->TemplateArguments;
-      LV = merge(LV, getLVForTemplateArgumentList(TemplateArgs));
-
-      if (SpecInfo->getTemplateSpecializationKind()
-            == TSK_ExplicitInstantiationDeclaration)
-        ConsiderGlobalVisibility = false;
+      LV.merge(getLVForTemplateArgumentList(TemplateArgs));
     }
 
-    // -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
   //       for linkage purposes (7.1.3); or
@@ -345,7 +332,7 @@
   } else if (const TagDecl *Tag = dyn_cast<TagDecl>(D)) {
     // Unnamed tags have no linkage.
     if (!Tag->getDeclName() && !Tag->getTypedefForAnonDecl())
-      return LVPair(NoLinkage, DefaultVisibility);
+      return LinkageInfo::none();
 
     // If this is a class template specialization, consider the
     // linkage of the template and template arguments.
@@ -353,15 +340,11 @@
           = dyn_cast<ClassTemplateSpecializationDecl>(Tag)) {
       // From the template.  Note below the restrictions on how we
       // compute template visibility.
-      LV = merge(LV, Spec->getSpecializedTemplate()->getLinkageAndVisibility());
+      LV.merge(Spec->getSpecializedTemplate()->getLinkageAndVisibility());
 
       // The arguments at which the template was instantiated.
       const TemplateArgumentList &TemplateArgs = Spec->getTemplateArgs();
-      LV = merge(LV, getLVForTemplateArgumentList(TemplateArgs));
-
-      if (Spec->getTemplateSpecializationKind()
-            == TSK_ExplicitInstantiationDeclaration)
-        ConsiderGlobalVisibility = false;
+      LV.merge(getLVForTemplateArgumentList(TemplateArgs));
     }
 
     // Consider -fvisibility unless the type has C linkage.
@@ -372,17 +355,16 @@
 
   //     - an enumerator belonging to an enumeration with external linkage;
   } else if (isa<EnumConstantDecl>(D)) {
-    LVPair EnumLV =
+    LinkageInfo EnumLV =
       cast<NamedDecl>(D->getDeclContext())->getLinkageAndVisibility();
-    if (!isExternalLinkage(EnumLV.first))
-      return LVPair(NoLinkage, DefaultVisibility);
-    LV = merge(LV, EnumLV);
+    if (!isExternalLinkage(EnumLV.linkage()))
+      return LinkageInfo::none();
+    LV.merge(EnumLV);
 
   //     - a template, unless it is a function template that has
   //       internal linkage (Clause 14);
   } else if (const TemplateDecl *Template = dyn_cast<TemplateDecl>(D)) {
-    LV = merge(LV, getLVForTemplateParameterList(
-                                         Template->getTemplateParameters()));
+    LV.merge(getLVForTemplateParameterList(Template->getTemplateParameters()));
 
     // We do not want to consider attributes or global settings when
     // computing template visibility.
@@ -400,35 +382,25 @@
 
   // Everything not covered here has no linkage.
   } else {
-    return LVPair(NoLinkage, DefaultVisibility);
+    return LinkageInfo::none();
   }
 
   // If we ended up with non-external linkage, visibility should
   // always be default.
-  if (LV.first != ExternalLinkage)
-    return LVPair(LV.first, DefaultVisibility);
+  if (LV.linkage() != ExternalLinkage)
+    return LinkageInfo(LV.linkage(), DefaultVisibility, false);
 
   // If we didn't end up with hidden visibility, consider attributes
   // and -fvisibility.
-  if (LV.second != HiddenVisibility) {
-    Visibility StandardV;
-
-    // If we have an explicit visibility attribute, merge that in.
-    if (ExplicitVisibility)
-      StandardV = GetVisibilityFromAttr(ExplicitVisibility);
-    else if (ConsiderGlobalVisibility)
-      StandardV = Context.getLangOptions().getVisibilityMode();
-    else
-      StandardV = DefaultVisibility; // no-op
-
-    LV.second = minVisibility(LV.second, StandardV);  
-  }
+  if (ConsiderGlobalVisibility && !LV.visibilityExplicit() &&
+      LV.visibility() != HiddenVisibility)
+    LV.mergeVisibility(Context.getLangOptions().getVisibilityMode());
 
   return LV;
 }
 
-static LVPair getLVForClassMember(const NamedDecl *D,
-                                  bool ConsiderGlobalVisibility) {
+static LinkageInfo getLVForClassMember(const NamedDecl *D,
+                                       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
@@ -438,31 +410,30 @@
         isa<FieldDecl>(D) ||
         (isa<TagDecl>(D) &&
          (D->getDeclName() || cast<TagDecl>(D)->getTypedefForAnonDecl()))))
-    return LVPair(NoLinkage, DefaultVisibility);
+    return LinkageInfo::none();
 
-  // Class members only have linkage if their class has external linkage.
-  // Always ignore global visibility settings during this.
-  LVPair ClassLV = getLVForDecl(cast<RecordDecl>(D->getDeclContext()), false);
-  if (!isExternalLinkage(ClassLV.first))
-    return LVPair(NoLinkage, DefaultVisibility);
+  const VisibilityAttr *VA = GetExplicitVisibility(D);
+  if (VA)
+    ConsiderGlobalVisibility = false;
+
+  // Class members only have linkage if their class has external
+  // linkage.  Consider global visibility only if we have no explicit
+  // visibility attributes.
+  LinkageInfo ClassLV = getLVForDecl(cast<RecordDecl>(D->getDeclContext()),
+                                     ConsiderGlobalVisibility);
+  if (!isExternalLinkage(ClassLV.linkage()))
+    return LinkageInfo::none();
 
   // If the class already has unique-external linkage, we can't improve.
-  if (ClassLV.first == UniqueExternalLinkage)
-    return LVPair(UniqueExternalLinkage, DefaultVisibility);
+  if (ClassLV.linkage() == UniqueExternalLinkage)
+    return LinkageInfo::uniqueExternal();
 
   // 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;
-  }
+  LinkageInfo LV = ClassLV;
 
-  bool HasExplicitVisibility = (VA ||
-    HasExplicitVisibilityInHierarchy(cast<RecordDecl>(D->getDeclContext())));
+  // If we have an explicit visibility attribute, merge that in.
+  if (VA)
+    LV.mergeVisibility(GetVisibilityFromAttr(VA), true);
 
   if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D)) {
     TemplateSpecializationKind TSK = TSK_Undeclared;
@@ -471,8 +442,8 @@
     // the template parameters and arguments.
     if (FunctionTemplateSpecializationInfo *Spec
            = MD->getTemplateSpecializationInfo()) {
-      LV = merge(LV, getLVForTemplateArgumentList(*Spec->TemplateArguments));
-      LV = merge(LV, getLVForTemplateParameterList(
+      LV.merge(getLVForTemplateArgumentList(*Spec->TemplateArguments));
+      LV.merge(getLVForTemplateParameterList(
                               Spec->getTemplate()->getTemplateParameters()));
 
       TSK = Spec->getTemplateSpecializationKind();
@@ -481,83 +452,57 @@
       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() &&
+    // Note that ConsiderGlobalVisibility doesn't yet have information
+    // about whether containing classes have visibility attributes,
+    // and that's intentional.
+    if (TSK != TSK_ExplicitInstantiationDeclaration &&
+        ConsiderGlobalVisibility && MD->isInlined() &&
         MD->getASTContext().getLangOptions().InlineVisibilityHidden)
-      LV.second = HiddenVisibility;
+      LV.setVisibility(HiddenVisibility);
 
     // 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(
+      LV.merge(getLVForTemplateArgumentList(Spec->getTemplateArgs()));
+      LV.merge(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)) {
-    bool IsDefinition = (VD->getDefinition() &&
-                         VD->getTemplateSpecializationKind()
-                           != TSK_ExplicitInstantiationDeclaration);
-
-    // GCC just ignores the visibility of a variable declaration
-    // unless it's explicit.
-    if (!IsDefinition && !HasExplicitVisibility) {
-      LV.second = DefaultVisibility;
-      ConsiderGlobalVisibility = false;
-    }
-
     // Modify the variable's linkage by its type, but ignore the
     // type's visibility unless it's a definition.
     LVPair TypeLV = VD->getType()->getLinkageAndVisibility();
     if (TypeLV.first != ExternalLinkage)
-      LV.first = minLinkage(LV.first, UniqueExternalLinkage);
-    if (IsDefinition && !HasExplicitVisibility)
-      LV.second = minVisibility(LV.second, TypeLV.second);
+      LV.mergeLinkage(UniqueExternalLinkage);
+    if (!LV.visibilityExplicit())
+      LV.mergeVisibility(TypeLV.second);
   }
 
-  // Suppress -fvisibility if we have explicit visibility on any of
-  // our ancestors.
-  ConsiderGlobalVisibility &= !HasExplicitVisibility;
+  ConsiderGlobalVisibility &= !LV.visibilityExplicit();
 
   // Apply -fvisibility if desired.
-  if (ConsiderGlobalVisibility && LV.second != HiddenVisibility) {
-    LV.second = minVisibility(LV.second,
-                    D->getASTContext().getLangOptions().getVisibilityMode());
+  if (ConsiderGlobalVisibility && LV.visibility() != HiddenVisibility) {
+    LV.mergeVisibility(D->getASTContext().getLangOptions().getVisibilityMode());
   }
 
   return LV;
 }
 
-LVPair NamedDecl::getLinkageAndVisibility() const {
+LinkageInfo NamedDecl::getLinkageAndVisibility() const {
   return getLVForDecl(this, /*ConsiderGlobalSettings*/ true);
 }
 
-static LVPair getLVForDecl(const NamedDecl *D, bool ConsiderGlobalSettings) {
+static LinkageInfo getLVForDecl(const NamedDecl *D,
+                                bool ConsiderGlobalVisibility) {
   // Objective-C: treat all Objective-C declarations as having external
   // linkage.
   switch (D->getKind()) {
@@ -575,12 +520,12 @@
     case Decl::ObjCProperty:
     case Decl::ObjCPropertyImpl:
     case Decl::ObjCProtocol:
-      return LVPair(ExternalLinkage, DefaultVisibility);
+      return LinkageInfo::external();
   }
 
   // Handle linkage for namespace-scope names.
   if (D->getDeclContext()->getRedeclContext()->isFileContext())
-    return getLVForNamespaceScopeDecl(D, ConsiderGlobalSettings);
+    return getLVForNamespaceScopeDecl(D, ConsiderGlobalVisibility);
   
   // C++ [basic.link]p5:
   //   In addition, a member function, static data member, a named
@@ -590,7 +535,7 @@
   //   purposes (7.1.3), has external linkage if the name of the class
   //   has external linkage.
   if (D->getDeclContext()->isRecord())
-    return getLVForClassMember(D, ConsiderGlobalSettings);
+    return getLVForClassMember(D, ConsiderGlobalVisibility);
 
   // C++ [basic.link]p6:
   //   The name of a function declared in block scope and the name of
@@ -606,16 +551,16 @@
   if (D->getLexicalDeclContext()->isFunctionOrMethod()) {
     if (const FunctionDecl *Function = dyn_cast<FunctionDecl>(D)) {
       if (Function->isInAnonymousNamespace())
-        return LVPair(UniqueExternalLinkage, DefaultVisibility);
+        return LinkageInfo::uniqueExternal();
 
-      LVPair LV(ExternalLinkage, DefaultVisibility);
+      LinkageInfo LV;
       if (const VisibilityAttr *VA = GetExplicitVisibility(Function))
-        LV.second = GetVisibilityFromAttr(VA);
+        LV.setVisibility(GetVisibilityFromAttr(VA));
 
       if (const FunctionDecl *Prev = Function->getPreviousDeclaration()) {
-        LVPair PrevLV = Prev->getLinkageAndVisibility();
-        if (PrevLV.first) LV.first = PrevLV.first;
-        LV.second = minVisibility(LV.second, PrevLV.second);
+        LinkageInfo PrevLV = Prev->getLinkageAndVisibility();
+        if (PrevLV.linkage()) LV.setLinkage(PrevLV.linkage());
+        LV.mergeVisibility(PrevLV);
       }
 
       return LV;
@@ -625,18 +570,18 @@
       if (Var->getStorageClass() == SC_Extern ||
           Var->getStorageClass() == SC_PrivateExtern) {
         if (Var->isInAnonymousNamespace())
-          return LVPair(UniqueExternalLinkage, DefaultVisibility);
+          return LinkageInfo::uniqueExternal();
 
-        LVPair LV(ExternalLinkage, DefaultVisibility);
+        LinkageInfo LV;
         if (Var->getStorageClass() == SC_PrivateExtern)
-          LV.second = HiddenVisibility;
+          LV.setVisibility(HiddenVisibility);
         else if (const VisibilityAttr *VA = GetExplicitVisibility(Var))
-          LV.second = GetVisibilityFromAttr(VA);
+          LV.setVisibility(GetVisibilityFromAttr(VA));
 
         if (const VarDecl *Prev = Var->getPreviousDeclaration()) {
-          LVPair PrevLV = Prev->getLinkageAndVisibility();
-          if (PrevLV.first) LV.first = PrevLV.first;
-          LV.second = minVisibility(LV.second, PrevLV.second);
+          LinkageInfo PrevLV = Prev->getLinkageAndVisibility();
+          if (PrevLV.linkage()) LV.setLinkage(PrevLV.linkage());
+          LV.mergeVisibility(PrevLV);
         }
 
         return LV;
@@ -645,7 +590,7 @@
 
   // C++ [basic.link]p6:
   //   Names not covered by these rules have no linkage.
-  return LVPair(NoLinkage, DefaultVisibility);
+  return LinkageInfo::none();
 }
 
 std::string NamedDecl::getQualifiedNameAsString() const {

Modified: cfe/trunk/lib/AST/Type.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=117781&r1=117780&r2=117781&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Sat Oct 30 06:50:40 2010
@@ -1352,12 +1352,12 @@
   //       linkage purposes (7.1.3)) and the name has linkage; or
   //     -  it is a specialization of a class template (14); or
 
-  std::pair<Linkage,Visibility> LV = getDecl()->getLinkageAndVisibility();
+  NamedDecl::LinkageInfo LV = getDecl()->getLinkageAndVisibility();
   bool IsLocalOrUnnamed =
     getDecl()->getDeclContext()->isFunctionOrMethod() ||
                         (!getDecl()->getIdentifier() &&
                          !getDecl()->getTypedefForAnonDecl());
-  return CachedProperties(LV.first, LV.second, IsLocalOrUnnamed);
+  return CachedProperties(LV.linkage(), LV.visibility(), IsLocalOrUnnamed);
 }
 
 // C++ [basic.link]p8:
@@ -1406,8 +1406,8 @@
 }
 
 Type::CachedProperties ObjCInterfaceType::getCachedProperties() const {
-  std::pair<Linkage,Visibility> LV = getDecl()->getLinkageAndVisibility();
-  return CachedProperties(LV.first, LV.second, false);
+  NamedDecl::LinkageInfo LV = getDecl()->getLinkageAndVisibility();
+  return CachedProperties(LV.linkage(), LV.visibility(), false);
 }
 
 Type::CachedProperties ObjCObjectType::getCachedProperties() const {

Modified: cfe/trunk/lib/CodeGen/CGRTTI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGRTTI.cpp?rev=117781&r1=117780&r2=117781&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGRTTI.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGRTTI.cpp Sat Oct 30 06:50:40 2010
@@ -637,7 +637,7 @@
   // compatibility.
   if (const RecordType *RT = dyn_cast<RecordType>(Ty))
     CGM.setTypeVisibility(GV, cast<CXXRecordDecl>(RT->getDecl()),
-                          /*ForRTTI*/ true);
+                          /*ForRTTI*/ true, /*ForDefinition*/ true);
   else if (CGM.getCodeGenOpts().HiddenWeakVTables &&
            Linkage == llvm::GlobalValue::WeakODRLinkage)
     GV->setVisibility(llvm::GlobalValue::HiddenVisibility);

Modified: cfe/trunk/lib/CodeGen/CGVTT.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTT.cpp?rev=117781&r1=117780&r2=117781&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGVTT.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTT.cpp Sat Oct 30 06:50:40 2010
@@ -397,7 +397,7 @@
     llvm::GlobalVariable *OldGV = GV;
     GV = new llvm::GlobalVariable(CGM.getModule(), Type, /*isConstant=*/true, 
                                   Linkage, Init, Name);
-    CGM.setGlobalVisibility(GV, RD);
+    CGM.setGlobalVisibility(GV, RD, /*ForDefinition*/ GenerateDefinition);
     
     if (OldGV) {
       GV->takeName(OldGV);

Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=117781&r1=117780&r2=117781&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp Sat Oct 30 06:50:40 2010
@@ -2485,7 +2485,7 @@
 
 static void setThunkVisibility(CodeGenModule &CGM, const CXXMethodDecl *MD,
                                const ThunkInfo &Thunk, llvm::Function *Fn) {
-  CGM.setGlobalVisibility(Fn, MD);
+  CGM.setGlobalVisibility(Fn, MD, /*ForDef*/ true);
 
   if (!CGM.getCodeGenOpts().HiddenWeakVTables)
     return;
@@ -2989,7 +2989,7 @@
   VTable->setLinkage(Linkage);
   
   // Set the right visibility.
-  CGM.setTypeVisibility(VTable, RD, /*ForRTTI*/ false);
+  CGM.setTypeVisibility(VTable, RD, /*ForRTTI*/ false, /*ForDef*/ true);
 }
 
 llvm::GlobalVariable *

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=117781&r1=117780&r2=117781&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Sat Oct 30 06:50:40 2010
@@ -176,22 +176,28 @@
 
 
 void CodeGenModule::setGlobalVisibility(llvm::GlobalValue *GV,
-                                        const NamedDecl *D) const {
+                                        const NamedDecl *D,
+                                        bool IsForDefinition) const {
   // Internal definitions always have default visibility.
   if (GV->hasLocalLinkage()) {
     GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
     return;
   }
 
-  GV->setVisibility(GetLLVMVisibility(D->getVisibility()));
+  // Set visibility for definitions.
+  NamedDecl::LinkageInfo LV = D->getLinkageAndVisibility();
+  if (LV.visibilityExplicit() ||
+      (IsForDefinition && !GV->hasAvailableExternallyLinkage()))
+    GV->setVisibility(GetLLVMVisibility(LV.visibility()));
 }
 
 /// Set the symbol visibility of type information (vtable and RTTI)
 /// associated with the given type.
 void CodeGenModule::setTypeVisibility(llvm::GlobalValue *GV,
                                       const CXXRecordDecl *RD,
-                                      bool IsForRTTI) const {
-  setGlobalVisibility(GV, RD);
+                                      bool IsForRTTI,
+                                      bool IsForDefinition) const {
+  setGlobalVisibility(GV, RD, IsForDefinition);
 
   if (!CodeGenOpts.HiddenWeakVTables)
     return;
@@ -444,7 +450,7 @@
 void CodeGenModule::SetCommonAttributes(const Decl *D,
                                         llvm::GlobalValue *GV) {
   if (isa<NamedDecl>(D))
-    setGlobalVisibility(GV, cast<NamedDecl>(D));
+    setGlobalVisibility(GV, cast<NamedDecl>(D), /*ForDef*/ true);
   else
     GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
 
@@ -488,6 +494,11 @@
     F->setLinkage(llvm::Function::ExternalWeakLinkage);
   } else {
     F->setLinkage(llvm::Function::ExternalLinkage);
+
+    NamedDecl::LinkageInfo LV = FD->getLinkageAndVisibility();
+    if (LV.linkage() == ExternalLinkage && LV.visibilityExplicit()) {
+      F->setVisibility(GetLLVMVisibility(LV.visibility()));
+    }
   }
 
   if (const SectionAttr *SA = FD->getAttr<SectionAttr>())
@@ -822,11 +833,11 @@
     // If this the first reference to a C++ inline function in a class, queue up
     // the deferred function body for emission.  These are not seen as
     // top-level declarations.
-    if (FD->isThisDeclarationADefinition() && MayDeferGeneration(FD))
+    if (FD->isThisDeclarationADefinition() && MayDeferGeneration(FD)) {
       DeferredDeclsToEmit.push_back(D);
     // A called constructor which has no definition or declaration need be
     // synthesized.
-    else if (const CXXConstructorDecl *CD = dyn_cast<CXXConstructorDecl>(FD)) {
+    } else if (const CXXConstructorDecl *CD = dyn_cast<CXXConstructorDecl>(FD)) {
       if (CD->isImplicit()) {
         assert(CD->isUsed() && "Sema doesn't consider constructor as used.");
         DeferredDeclsToEmit.push_back(D);
@@ -938,8 +949,8 @@
     GV->setConstant(DeclIsConstantGlobal(Context, D));
 
     // Set linkage and visibility in case we never see a definition.
-    std::pair<Linkage,Visibility> LV = D->getLinkageAndVisibility();
-    if (LV.first != ExternalLinkage) {
+    NamedDecl::LinkageInfo LV = D->getLinkageAndVisibility();
+    if (LV.linkage() != ExternalLinkage) {
       GV->setLinkage(llvm::GlobalValue::InternalLinkage);
     } else {
       if (D->hasAttr<DLLImportAttr>())
@@ -947,7 +958,9 @@
       else if (D->hasAttr<WeakAttr>() || D->hasAttr<WeakImportAttr>())
         GV->setLinkage(llvm::GlobalValue::ExternalWeakLinkage);
 
-      GV->setVisibility(GetLLVMVisibility(LV.second));
+      // Set visibility on a declaration only if it's explicit.
+      if (LV.visibilityExplicit())
+        GV->setVisibility(GetLLVMVisibility(LV.visibility()));
     }
 
     GV->setThreadLocal(D->isThreadSpecified());

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=117781&r1=117780&r2=117781&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.h Sat Oct 30 06:50:40 2010
@@ -259,12 +259,13 @@
 
   /// setGlobalVisibility - Set the visibility for the given LLVM
   /// GlobalValue.
-  void setGlobalVisibility(llvm::GlobalValue *GV, const NamedDecl *D) const;
+  void setGlobalVisibility(llvm::GlobalValue *GV, const NamedDecl *D,
+                           bool IsForDefinition) const;
 
   /// setTypeVisibility - Set the visibility for the given global
   /// value which holds information about a type.
   void setTypeVisibility(llvm::GlobalValue *GV, const CXXRecordDecl *D,
-                         bool IsForRTTI) const;
+                         bool IsForRTTI, bool IsForDefinition) const;
 
   llvm::Constant *GetAddrOfGlobal(GlobalDecl GD) {
     if (isa<CXXConstructorDecl>(GD.getDecl()))

Modified: cfe/trunk/test/CodeGenCXX/visibility.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/visibility.cpp?rev=117781&r1=117780&r2=117781&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/visibility.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/visibility.cpp Sat Oct 30 06:50:40 2010
@@ -22,6 +22,8 @@
 // CHECK-HIDDEN: @_ZN6Test143varE = external global
 // CHECK: @_ZN6Test154TempINS_1AEE5Inner6bufferE = external global [0 x i8]
 // CHECK-HIDDEN: @_ZN6Test154TempINS_1AEE5Inner6bufferE = external global [0 x i8]
+// CHECK-HIDDEN: @_ZTTN6Test161AIcEE = external constant
+// CHECK-HIDDEN: @_ZTVN6Test161AIcEE = external constant
 // CHECK: @_ZTVN5Test63fooE = weak_odr hidden constant 
 
 namespace Test1 {
@@ -234,3 +236,17 @@
     return Temp<A>::Inner::buffer;
   }
 }
+
+namespace Test16 {
+  struct Base1 { virtual void foo(); };
+  struct Base2 : virtual Base1 { virtual void foo(); };
+  template <class T> struct A : virtual Base1, Base2 {
+    virtual void foo();
+  };
+  extern template struct A<char>;
+
+  void test() {
+    A<char> a;
+    a.foo();
+  }
+}





More information about the cfe-commits mailing list