[cfe-commits] r148163 - in /cfe/trunk: include/clang/AST/Decl.h include/clang/AST/Type.h lib/AST/Decl.cpp lib/AST/Type.cpp test/CodeGenCXX/visibility.cpp

Rafael Espindola rafael.espindola at gmail.com
Fri Jan 13 16:30:36 PST 2012


Author: rafael
Date: Fri Jan 13 18:30:36 2012
New Revision: 148163

URL: http://llvm.org/viewvc/llvm-project?rev=148163&view=rev
Log:
Remember if a type has its visibility set explicitly or implicitly.
With that, centralize the way we merge visibility, always preferring explicit over
implicit and then picking the most restrictive one.
Fixes pr10113 and pr11690.

Modified:
    cfe/trunk/include/clang/AST/Decl.h
    cfe/trunk/include/clang/AST/Type.h
    cfe/trunk/lib/AST/Decl.cpp
    cfe/trunk/lib/AST/Type.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=148163&r1=148162&r2=148163&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Fri Jan 13 18:30:36 2012
@@ -252,6 +252,14 @@
     }
 
     void mergeVisibility(Visibility V, bool E = false) {
+      // If one has explicit visibility and the other doesn't, keep the
+      // explicit one.
+      if (visibilityExplicit() && !E)
+        return;
+      if (!visibilityExplicit() && E)
+        setVisibility(V, E);
+
+      // If both are explicit or both are implicit, keep the minimum.
       setVisibility(minVisibility(visibility(), V), visibilityExplicit() || E);
     }
     void mergeVisibility(LinkageInfo Other) {
@@ -262,10 +270,6 @@
       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);

Modified: cfe/trunk/include/clang/AST/Type.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=148163&r1=148162&r2=148163&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Fri Jan 13 18:30:36 2012
@@ -1088,6 +1088,9 @@
     /// LangOptions::VisibilityMode+1.
     mutable unsigned CacheValidAndVisibility : 2;
 
+    /// \brief True if the visibility was set explicitly in the source code.
+    mutable unsigned CachedExplicitVisibility : 1;
+
     /// \brief Linkage of this type.
     mutable unsigned CachedLinkage : 2;
 
@@ -1104,6 +1107,10 @@
       assert(isCacheValid() && "getting linkage from invalid cache");
       return static_cast<Visibility>(CacheValidAndVisibility-1);
     }
+    bool isVisibilityExplicit() const {
+      assert(isCacheValid() && "getting linkage from invalid cache");
+      return CachedExplicitVisibility;
+    }
     Linkage getLinkage() const {
       assert(isCacheValid() && "getting linkage from invalid cache");
       return static_cast<Linkage>(CachedLinkage);
@@ -1113,7 +1120,7 @@
       return CachedLocalOrUnnamed;
     }
   };
-  enum { NumTypeBits = 18 };
+  enum { NumTypeBits = 19 };
 
 protected:
   // These classes allow subclasses to somewhat cleanly pack bitfields
@@ -1267,6 +1274,7 @@
     TypeBits.VariablyModified = VariablyModified;
     TypeBits.ContainsUnexpandedParameterPack = ContainsUnexpandedParameterPack;
     TypeBits.CacheValidAndVisibility = 0;
+    TypeBits.CachedExplicitVisibility = false;
     TypeBits.CachedLocalOrUnnamed = false;
     TypeBits.CachedLinkage = NoLinkage;
     TypeBits.FromAST = false;
@@ -1643,6 +1651,9 @@
   /// \brief Determine the visibility of this type.
   Visibility getVisibility() const;
 
+  /// \brief Return true if the visibility was explicitly set is the code.
+  bool isVisibilityExplicit() const;
+
   /// \brief Determine the linkage and visibility of this type.
   std::pair<Linkage,Visibility> getLinkageAndVisibility() const;
 

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=148163&r1=148162&r2=148163&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Fri Jan 13 18:30:36 2012
@@ -67,17 +67,6 @@
 }
 
 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()));
-}
 
 namespace {
 /// Flags controlling the computation of linkage and visibility.
@@ -113,11 +102,16 @@
 }; 
 } // end anonymous namespace
 
+static LinkageInfo getLVForType(QualType T) {
+  std::pair<Linkage,Visibility> P = T->getLinkageAndVisibility();
+  return LinkageInfo(P.first, P.second, T->isVisibilityExplicit());
+}
+
 /// \brief Get the most restrictive linkage for the types in the given
 /// template parameter list.
-static LVPair
+static LinkageInfo
 getLVForTemplateParameterList(const TemplateParameterList *Params) {
-  LVPair LV(ExternalLinkage, DefaultVisibility);
+  LinkageInfo LV(ExternalLinkage, DefaultVisibility, false);
   for (TemplateParameterList::const_iterator P = Params->begin(),
                                           PEnd = Params->end();
        P != PEnd; ++P) {
@@ -126,20 +120,20 @@
         for (unsigned I = 0, N = NTTP->getNumExpansionTypes(); I != N; ++I) {
           QualType T = NTTP->getExpansionType(I);
           if (!T->isDependentType())
-            LV = merge(LV, T->getLinkageAndVisibility());
+            LV.merge(getLVForType(T));
         }
         continue;
       }
 
       if (!NTTP->getType()->isDependentType()) {
-        LV = merge(LV, NTTP->getType()->getLinkageAndVisibility());
+        LV.merge(getLVForType(NTTP->getType()));
         continue;
       }
     }
 
     if (TemplateTemplateParmDecl *TTP
                                    = dyn_cast<TemplateTemplateParmDecl>(*P)) {
-      LV = merge(LV, getLVForTemplateParameterList(TTP->getTemplateParameters()));
+      LV.merge(getLVForTemplateParameterList(TTP->getTemplateParameters()));
     }
   }
 
@@ -151,10 +145,10 @@
 
 /// \brief Get the most restrictive linkage for the types and
 /// declarations in the given template argument list.
-static LVPair getLVForTemplateArgumentList(const TemplateArgument *Args,
-                                           unsigned NumArgs,
-                                           LVFlags &F) {
-  LVPair LV(ExternalLinkage, DefaultVisibility);
+static LinkageInfo getLVForTemplateArgumentList(const TemplateArgument *Args,
+                                                unsigned NumArgs,
+                                                LVFlags &F) {
+  LinkageInfo LV(ExternalLinkage, DefaultVisibility, false);
 
   for (unsigned I = 0; I != NumArgs; ++I) {
     switch (Args[I].getKind()) {
@@ -164,7 +158,7 @@
       break;
 
     case TemplateArgument::Type:
-      LV = merge(LV, Args[I].getAsType()->getLinkageAndVisibility());
+      LV.merge(getLVForType(Args[I].getAsType()));
       break;
 
     case TemplateArgument::Declaration:
@@ -180,13 +174,13 @@
     case TemplateArgument::TemplateExpansion:
       if (TemplateDecl *Template
                 = Args[I].getAsTemplateOrTemplatePattern().getAsTemplateDecl())
-        LV = merge(LV, getLVForDecl(Template, F));
+        LV.merge(getLVForDecl(Template, F));
       break;
 
     case TemplateArgument::Pack:
-      LV = merge(LV, getLVForTemplateArgumentList(Args[I].pack_begin(),
-                                                  Args[I].pack_size(),
-                                                  F));
+      LV.merge(getLVForTemplateArgumentList(Args[I].pack_begin(),
+                                            Args[I].pack_size(),
+                                            F));
       break;
     }
   }
@@ -194,7 +188,7 @@
   return LV;
 }
 
-static LVPair
+static LinkageInfo
 getLVForTemplateArgumentList(const TemplateArgumentList &TArgs,
                              LVFlags &F) {
   return getLVForTemplateArgumentList(TArgs.data(), TArgs.size(), F);
@@ -337,11 +331,11 @@
     // 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()) {
-      LVPair TypeLV = Var->getType()->getLinkageAndVisibility();
-      if (TypeLV.first != ExternalLinkage)
+      LinkageInfo TypeLV = getLVForType(Var->getType());
+      if (TypeLV.linkage() != ExternalLinkage)
         return LinkageInfo::uniqueExternal();
       if (!LV.visibilityExplicit())
-        LV.mergeVisibility(TypeLV.second);
+        LV.mergeVisibility(TypeLV.visibility(), TypeLV.visibilityExplicit());
     }
 
     if (Var->getStorageClass() == SC_PrivateExtern)
@@ -599,11 +593,11 @@
   } else if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
     // 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)
+    LinkageInfo TypeLV = getLVForType(VD->getType());
+    if (TypeLV.linkage() != ExternalLinkage)
       LV.mergeLinkage(UniqueExternalLinkage);
     if (!LV.visibilityExplicit())
-      LV.mergeVisibility(TypeLV.second);
+      LV.mergeVisibility(TypeLV.visibility(), TypeLV.visibilityExplicit());
   }
 
   F.ConsiderGlobalVisibility &= !LV.visibilityExplicit();

Modified: cfe/trunk/lib/AST/Type.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=148163&r1=148162&r2=148163&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Fri Jan 13 18:30:36 2012
@@ -1990,21 +1990,22 @@
 
 /// \brief The cached properties of a type.
 class CachedProperties {
-  char linkage;
-  char visibility;
+  NamedDecl::LinkageInfo LV;
   bool local;
   
 public:
-  CachedProperties(Linkage linkage, Visibility visibility, bool local)
-    : linkage(linkage), visibility(visibility), local(local) {}
+  CachedProperties(NamedDecl::LinkageInfo LV, bool local)
+    : LV(LV), local(local) {}
   
-  Linkage getLinkage() const { return (Linkage) linkage; }
-  Visibility getVisibility() const { return (Visibility) visibility; }
+  Linkage getLinkage() const { return LV.linkage(); }
+  Visibility getVisibility() const { return LV.visibility(); }
+  bool isVisibilityExplicit() const { return LV.visibilityExplicit(); }
   bool hasLocalOrUnnamedType() const { return local; }
   
   friend CachedProperties merge(CachedProperties L, CachedProperties R) {
-    return CachedProperties(minLinkage(L.getLinkage(), R.getLinkage()),
-                            minVisibility(L.getVisibility(), R.getVisibility()),
+    NamedDecl::LinkageInfo MergedLV = L.LV;
+    MergedLV.merge(R.LV);
+    return CachedProperties(MergedLV,
                          L.hasLocalOrUnnamedType() | R.hasLocalOrUnnamedType());
   }
 };
@@ -2024,9 +2025,10 @@
 
   static CachedProperties get(const Type *T) {
     ensure(T);
-    return CachedProperties(T->TypeBits.getLinkage(),
-                            T->TypeBits.getVisibility(),
-                            T->TypeBits.hasLocalOrUnnamedType());
+    NamedDecl::LinkageInfo LV(T->TypeBits.getLinkage(),
+                              T->TypeBits.getVisibility(),
+                              T->TypeBits.isVisibilityExplicit());
+    return CachedProperties(LV, T->TypeBits.hasLocalOrUnnamedType());
   }
 
   static void ensure(const Type *T) {
@@ -2040,6 +2042,8 @@
       ensure(CT);
       T->TypeBits.CacheValidAndVisibility =
         CT->TypeBits.CacheValidAndVisibility;
+      T->TypeBits.CachedExplicitVisibility =
+        CT->TypeBits.CachedExplicitVisibility;
       T->TypeBits.CachedLinkage = CT->TypeBits.CachedLinkage;
       T->TypeBits.CachedLocalOrUnnamed = CT->TypeBits.CachedLocalOrUnnamed;
       return;
@@ -2048,6 +2052,7 @@
     // Compute the cached properties and then set the cache.
     CachedProperties Result = computeCachedProperties(T);
     T->TypeBits.CacheValidAndVisibility = Result.getVisibility() + 1U;
+    T->TypeBits.CachedExplicitVisibility = Result.isVisibilityExplicit();
     assert(T->TypeBits.isCacheValid() &&
            T->TypeBits.getVisibility() == Result.getVisibility());
     T->TypeBits.CachedLinkage = Result.getLinkage();
@@ -2075,13 +2080,13 @@
 #include "clang/AST/TypeNodes.def"
     // Treat instantiation-dependent types as external.
     assert(T->isInstantiationDependentType());
-    return CachedProperties(ExternalLinkage, DefaultVisibility, false);
+    return CachedProperties(NamedDecl::LinkageInfo(), false);
 
   case Type::Builtin:
     // C++ [basic.link]p8:
     //   A type is said to have linkage if and only if:
     //     - it is a fundamental type (3.9.1); or
-    return CachedProperties(ExternalLinkage, DefaultVisibility, false);
+    return CachedProperties(NamedDecl::LinkageInfo(), false);
 
   case Type::Record:
   case Type::Enum: {
@@ -2095,7 +2100,7 @@
     bool IsLocalOrUnnamed =
       Tag->getDeclContext()->isFunctionOrMethod() ||
       (!Tag->getIdentifier() && !Tag->getTypedefNameForAnonDecl());
-    return CachedProperties(LV.linkage(), LV.visibility(), IsLocalOrUnnamed);
+    return CachedProperties(LV, IsLocalOrUnnamed);
   }
 
     // C++ [basic.link]p8:
@@ -2135,7 +2140,7 @@
   case Type::ObjCInterface: {
     NamedDecl::LinkageInfo LV =
       cast<ObjCInterfaceType>(T)->getDecl()->getLinkageAndVisibility();
-    return CachedProperties(LV.linkage(), LV.visibility(), false);
+    return CachedProperties(LV, false);
   }
   case Type::ObjCObject:
     return Cache::get(cast<ObjCObjectType>(T)->getBaseType());
@@ -2149,7 +2154,8 @@
 
   // C++ [basic.link]p8:
   //   Names not covered by these rules have no linkage.
-  return CachedProperties(NoLinkage, DefaultVisibility, false);
+  NamedDecl::LinkageInfo LV(NoLinkage, DefaultVisibility, false);
+  return CachedProperties(LV, false);
 }
 
 /// \brief Determine the linkage of this type.
@@ -2164,6 +2170,11 @@
   return TypeBits.getVisibility();
 }
 
+bool Type::isVisibilityExplicit() const {
+  Cache::ensure(this);
+  return TypeBits.isVisibilityExplicit();
+}
+
 bool Type::hasUnnamedOrLocalType() const {
   Cache::ensure(this);
   return TypeBits.hasLocalOrUnnamedType();

Modified: cfe/trunk/test/CodeGenCXX/visibility.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/visibility.cpp?rev=148163&r1=148162&r2=148163&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/visibility.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/visibility.cpp Fri Jan 13 18:30:36 2012
@@ -403,10 +403,7 @@
     B<A<2> >::test4();
   }
 
-  // CHECK: declare void @_ZN6Test201BINS_1AILj2EEEE5test5Ev()
-  // (but explicit visibility on a template argument doesn't count as
-  //  explicit visibility for the template for purposes of deciding
-  //  whether an external symbol gets visibility)
+  // CHECK: declare hidden void @_ZN6Test201BINS_1AILj2EEEE5test5Ev()
   void test5() {
     B<A<2> >::test5();
   }
@@ -465,6 +462,12 @@
   template class foo::bar<char>;
   // CHECK: define weak_odr void @_ZN7PR101133foo3barIcE3zedEv
   // CHECK-HIDDEN: define weak_odr void @_ZN7PR101133foo3barIcE3zedEv
+
+  struct zed {
+  };
+  template class foo::bar<zed>;
+  // CHECK: define weak_odr void @_ZN7PR101133foo3barINS_3zedEE3zedEv
+  // CHECK-HIDDEN: define weak_odr void @_ZN7PR101133foo3barINS_3zedEE3zedEv
 }
 
 namespace PR11690 {
@@ -477,7 +480,23 @@
   // CHECK-HIDDEN: define weak_odr void @_ZNK7PR116905ClassIcE4sizeEv
 
   template<class T> void Method() {}
-  template  __attribute__((visibility("default"))) void Method<char>();
+  template  DEFAULT void Method<char>();
   // CHECK: define weak_odr void @_ZN7PR116906MethodIcEEvv
   // CHECK-HIDDEN: define weak_odr void @_ZN7PR116906MethodIcEEvv
 }
+
+namespace PR11690_2 {
+  namespace foo DEFAULT {
+    class bar;
+    template<typename T1, typename T2 = bar>
+    class zed {
+      void bar() {
+      }
+    };
+  }
+  struct baz {
+  };
+  template class foo::zed<baz>;
+  // CHECK: define weak_odr void @_ZN9PR11690_23foo3zedINS_3bazENS0_3barEE3barEv
+  // CHECK-HIDDEN: define weak_odr void @_ZN9PR11690_23foo3zedINS_3bazENS0_3barEE3barEv
+}





More information about the cfe-commits mailing list