Two fixes to how we compute visibility:

* Handle some situations where we should never make a decl more visible,
  even when merging in an explicit visibility.

* Handle attributes in members of classes that are explicitly specialized.

Thanks Nico for the report and testing, Eric for the initial review, and dgregor
for the awesome test27 :-)

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

diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h
index 760e598f75941c63fbaca1dd730a13a73f952c0f..c99ad115cfa48ea6c8fe9e7ed7cc3455b59535c4 100644 (file)
--- a/include/clang/AST/Decl.h
+++ b/include/clang/AST/Decl.h
@@ -251,6 +251,10 @@ public:
       mergeLinkage(Other.linkage());
     }
 
+    // Merge the visibility V giving preference to explicit ones.
+    // This is used, for example, when merging the visibility of a class
+    // down to one of its members. If the member has no explicit visibility,
+    // the class visibility wins.
     void mergeVisibility(Visibility V, bool E = false) {
       // If one has explicit visibility and the other doesn't, keep the
       // explicit one.
@@ -262,14 +266,35 @@ public:
       // If both are explicit or both are implicit, keep the minimum.
       setVisibility(minVisibility(visibility(), V), visibilityExplicit() || E);
     }
+    // Merge the visibility V, keeping the most restrictive one.
+    // This is used for cases like merging the visibility of a template
+    // argument to an instantiation. If we already have a hidden class,
+    // no argument should give it default visibility.
+    void mergeVisibilityWithMin(Visibility V, bool E = false) {
+      // Never increase the visibility
+      if (visibility() < V)
+        return;
+
+      // If this visibility is explicit, keep it.
+      if (visibilityExplicit() && !E)
+        return;
+      setVisibility(V, E);
+    }
     void mergeVisibility(LinkageInfo Other) {
       mergeVisibility(Other.visibility(), Other.visibilityExplicit());
     }
+    void mergeVisibilityWithMin(LinkageInfo Other) {
+      mergeVisibilityWithMin(Other.visibility(), Other.visibilityExplicit());
+    }
 
     void merge(LinkageInfo Other) {
       mergeLinkage(Other);
       mergeVisibility(Other);
     }
+    void mergeWithMin(LinkageInfo Other) {
+      mergeLinkage(Other);
+      mergeVisibilityWithMin(Other);
+    }
 
     friend LinkageInfo merge(LinkageInfo L, LinkageInfo R) {
       L.merge(R);

diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index 4d1b562c5930f35c72d97c7a5f13b32b3c95953f..433ce100cfd5270f58a70cfd210d5ac6562e88fc 100644 (file)
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -176,9 +176,9 @@ static LinkageInfo getLVForTemplateArgumentList(const TemplateArgument *Args,
       break;
 
     case TemplateArgument::Pack:
-      LV.merge(getLVForTemplateArgumentList(Args[I].pack_begin(),
-                                            Args[I].pack_size(),
-                                            F));
+      LV.mergeWithMin(getLVForTemplateArgumentList(Args[I].pack_begin(),
+                                                   Args[I].pack_size(),
+                                                   F));
       break;
     }
   }
@@ -279,6 +279,7 @@ static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, LVFlags F) {
   //   scope and no storage-class specifier, its linkage is
   //   external.
   LinkageInfo LV;
+  LV.mergeVisibility(Context.getLangOptions().getVisibilityMode());
 
   if (F.ConsiderVisibilityAttributes) {
     if (llvm::Optional<Visibility> Vis = D->getExplicitVisibility()) {
@@ -413,7 +414,7 @@ static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, LVFlags F) {
         LV.merge(getLVForDecl(specInfo->getTemplate(),
                               F.onlyTemplateVisibility()));
         const TemplateArgumentList &templateArgs = *specInfo->TemplateArguments;
-        LV.merge(getLVForTemplateArgumentList(templateArgs, F));
+        LV.mergeWithMin(getLVForTemplateArgumentList(templateArgs, F));
       }
     }
 
@@ -439,7 +440,7 @@ static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, LVFlags F) {
 
         // The arguments at which the template was instantiated.
         const TemplateArgumentList &TemplateArgs = spec->getTemplateArgs();
-        LV.merge(getLVForTemplateArgumentList(TemplateArgs, F));
+        LV.mergeWithMin(getLVForTemplateArgumentList(TemplateArgs, F));
       }
     }
 
@@ -482,11 +483,6 @@ static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, LVFlags F) {
   if (LV.linkage() != ExternalLinkage)
     return LinkageInfo(LV.linkage(), DefaultVisibility, false);
 
-  // If we didn't end up with hidden visibility, consider attributes
-  // and -fvisibility.
-  if (F.ConsiderGlobalVisibility)
-    LV.mergeVisibility(Context.getLangOptions().getVisibilityMode());
-
   return LV;
 }
 
@@ -503,6 +499,7 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D, LVFlags F) {
     return LinkageInfo::none();
 
   LinkageInfo LV;
+  LV.mergeVisibility(D->getASTContext().getLangOptions().getVisibilityMode());
 
   // The flags we're going to use to compute the class's visibility.
   LVFlags ClassF = F;
@@ -544,7 +541,8 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D, LVFlags F) {
     if (FunctionTemplateSpecializationInfo *spec
            = MD->getTemplateSpecializationInfo()) {
       if (shouldConsiderTemplateLV(MD, spec)) {
-        LV.merge(getLVForTemplateArgumentList(*spec->TemplateArguments, F));
+        LV.mergeWithMin(getLVForTemplateArgumentList(*spec->TemplateArguments,
+                                                     F));
         if (F.ConsiderTemplateParameterTypes)
           LV.merge(getLVForTemplateParameterList(
                               spec->getTemplate()->getTemplateParameters()));
@@ -583,7 +581,8 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D, LVFlags F) {
       if (shouldConsiderTemplateLV(spec)) {
         // Merge template argument/parameter information for member
         // class template specializations.
-        LV.merge(getLVForTemplateArgumentList(spec->getTemplateArgs(), F));
+        LV.mergeWithMin(getLVForTemplateArgumentList(spec->getTemplateArgs(),
+                                                     F));
       if (F.ConsiderTemplateParameterTypes)
         LV.merge(getLVForTemplateParameterList(
                     spec->getSpecializedTemplate()->getTemplateParameters()));
@@ -601,13 +600,6 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D, LVFlags F) {
       LV.mergeVisibility(TypeLV.visibility(), TypeLV.visibilityExplicit());
   }
 
-  F.ConsiderGlobalVisibility &= !LV.visibilityExplicit();
-
-  // Apply -fvisibility if desired.
-  if (F.ConsiderGlobalVisibility && LV.visibility() != HiddenVisibility) {
-    LV.mergeVisibility(D->getASTContext().getLangOptions().getVisibilityMode());
-  }
-
   return LV;
 }
 
@@ -697,6 +689,12 @@ llvm::Optional<Visibility> NamedDecl::getExplicitVisibility() const {
           = fn->getTemplateSpecializationInfo())
       return getVisibilityOf(templateInfo->getTemplate()->getTemplatedDecl());
 
+    // If the function is a member of a specialization of a class template
+    // and the corresponding decl has explicit visibility, use that.
+    FunctionDecl *InstantiatedFrom = fn->getInstantiatedFromMemberFunction();
+    if (InstantiatedFrom)
+      return getVisibilityOf(InstantiatedFrom);
+
     return llvm::Optional<Visibility>();
   }
 
@@ -711,6 +709,14 @@ llvm::Optional<Visibility> NamedDecl::getExplicitVisibility() const {
         = dyn_cast<ClassTemplateSpecializationDecl>(this))
     return getVisibilityOf(spec->getSpecializedTemplate()->getTemplatedDecl());
 
+  // If this is a member class of a specialization of a class template
+  // and the corresponding decl has explicit visibility, use that.
+  if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(this)) {
+    CXXRecordDecl *InstantiatedFrom = RD->getInstantiatedFromMemberClass();
+    if (InstantiatedFrom)
+      return getVisibilityOf(InstantiatedFrom);
+  }
+
   return llvm::Optional<Visibility>();
 }
 

diff --git a/lib/Sema/SemaTemplate.cpp b/lib/Sema/SemaTemplate.cpp
index 2c98d0ab97ba9495887bf34e330ee760f9f1a9b3..986a4c808171ac9339e730de3d08c0f019ef5158 100644 (file)
--- a/lib/Sema/SemaTemplate.cpp
+++ b/lib/Sema/SemaTemplate.cpp
@@ -5350,6 +5350,7 @@ Decl *Sema::ActOnStartOfFunctionTemplateDef(Scope *FnBodyScope,
 /// \brief Strips various properties off an implicit instantiation
 /// that has just been explicitly specialized.
 static void StripImplicitInstantiation(NamedDecl *D) {
+  // FIXME: "make check" is clean if the call to dropAttrs() is commented out.
   D->dropAttrs();
 
   if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {

diff --git a/test/CodeGenCXX/visibility.cpp b/test/CodeGenCXX/visibility.cpp
index 67ac0985331ce9a96bdef6ec85b5ce7145887d64..961b50844efbac5a23c2ee1b0ec2879eebe986f8 100644 (file)
--- a/test/CodeGenCXX/visibility.cpp
+++ b/test/CodeGenCXX/visibility.cpp
@@ -5,6 +5,21 @@
 #define PROTECTED __attribute__((visibility("protected")))
 #define DEFAULT __attribute__((visibility("default")))
 
+namespace test25 {
+  template<typename T>
+  struct X {
+    template<typename U>
+    struct definition {
+    };
+  };
+
+  class DEFAULT A { };
+
+  X<int>::definition<A> a;
+  // CHECK: @_ZN6test251aE = global
+  // CHECK-HIDDEN: @_ZN6test251aE = hidden global
+}
+
 // CHECK: @_ZN5Test425VariableInHiddenNamespaceE = hidden global i32 10
 // CHECK: @_ZN5Test71aE = hidden global
 // CHECK: @_ZN5Test71bE = global
@@ -22,6 +37,26 @@
 // CHECK-HIDDEN: @_ZN6Test143varE = external global
 // CHECK: @_ZN6Test154TempINS_1AEE5Inner6bufferE = external global [0 x i8]
 // CHECK-HIDDEN: @_ZN6Test154TempINS_1AEE5Inner6bufferE = external global [0 x i8]
+
+namespace test27 {
+  template<typename T>
+  class C {
+    class __attribute__((visibility("default"))) D {
+      void f();
+    };
+  };
+
+  template<>
+  class C<int>::D {
+    virtual void g();
+  };
+
+  void C<int>::D::g() {
+  }
+  // CHECK: _ZTVN6test271CIiE1DE = unnamed_addr constant
+  // CHECK-HIDDEN: _ZTVN6test271CIiE1DE = unnamed_addr constant
+}
+
 // CHECK: @_ZZN6Test193fooIiEEvvE1a = linkonce_odr global
 // CHECK: @_ZGVZN6Test193fooIiEEvvE1a = linkonce_odr global i64
 // CHECK-HIDDEN: @_ZZN6Test193fooIiEEvvE1a = linkonce_odr hidden global
@@ -500,3 +535,65 @@ namespace PR11690_2 {
   // CHECK: define weak_odr void @_ZN9PR11690_23foo3zedINS_3bazENS0_3barEE3barEv
   // CHECK-HIDDEN: define weak_odr void @_ZN9PR11690_23foo3zedINS_3bazENS0_3barEE3barEv
 }
+
+namespace test23 {
+  // Having a template argument that is explicitly visible should not make
+  // the template instantiation visible.
+  template <typename T>
+  struct X {
+    static void f() {
+    }
+  };
+
+  class DEFAULT A;
+
+  void g() {
+    X<A> y;
+    y.f();
+  }
+  // CHECK: define linkonce_odr void @_ZN6test231XINS_1AEE1fEv
+  // CHECK-HIDDEN: define linkonce_odr hidden void @_ZN6test231XINS_1AEE1fEv
+}
+
+namespace PR12001 {
+  template <typename P1>
+  void Bind(const P1& p1) {
+  }
+
+  class DEFAULT Version { };
+
+  void f() {
+    Bind(Version());
+  }
+  // CHECK: define linkonce_odr void @_ZN7PR120014BindINS_7VersionEEEvRKT_
+  // CHECK-HIDDEN: define linkonce_odr hidden void @_ZN7PR120014BindINS_7VersionEEEvRKT_
+}
+
+namespace test24 {
+  class DEFAULT A { };
+
+  struct S {
+    template <typename T>
+    void mem() {}
+  };
+
+  void test() {
+    S s;
+    s.mem<A>();
+  }
+  // CHECK: define linkonce_odr void @_ZN6test241S3memINS_1AEEEvv
+  // CHECK-HIDDEN: define linkonce_odr hidden void @_ZN6test241S3memINS_1AEEEvv
+}
+
+namespace test26 {
+  template<typename T>
+  class C {
+    __attribute__((visibility("default")))  void f();
+  };
+
+  template<>
+  void C<int>::f() { }
+
+  // CHECK: define void @_ZN6test261CIiE1fEv
+  // CHECK-HIDDEN: define void @_ZN6test261CIiE1fEv
+}