[Sema] Respect instantiated-from template's VisibilityAttr for two implicit/explicit instantiation cases

[MaskRay]

Summary

In CodeGenCXX/visibility.cpp,

Match GCC for part of test51: Change explicit instantiation of a function
template to respect the instantiated-from template's VisibilityAttr.

For the implicit instantiation zed<&z>(), our behavior remains different from
GCC, as the template parameter type 'foo' (hidden) wins
(shouldConsiderTemplateVisibility returns true). This seems an extreme corner
case.

Match GCC for test71: For an implicit or explicit instantiation of a class
template, change its member template instantiation to respect the
instantiated-from member template's VisibilityAttr.
Fix https://github.com/llvm/llvm-project/issues/31462

[MaskRay]

ping:)

[MaskRay]

Ping :)

[rjmccall]

clang/lib/AST/Decl.cpp

380

Okay, this change seems wrong. A visibility attribute on an explicit specialization or instantiation should definitely override everything else. A visibility attribute on a template pattern should only override other visibility information that we would derive from that pattern, like the visibility of the template parameters; it should not override visibility from the template arguments. You probably need this function to return an enum with three cases: (1) factor in both template arguments and template parameters, (2) factor in only template arguments, and (3) factor in nothing.

Also, the bug here doesn't seem to be specific to function templates, and you should fix the code for all three paths (function templates, class templates, and variable templates). Basically we're universally mishandling visibility attributes on template patterns for templates with non-type template parameters with hidden visibility.

[MaskRay]

rebase. this does not address rjmccall's comment, but just to show the difference to other tests

[MaskRay]

clang/lib/AST/Decl.cpp

380

I have added test coverage to test51 in visibility.cpp and studied GCC's behavior.

Sent https://github.com/llvm/llvm-project/pull/72092 to ignore visibility effects from template parameters. It will bring our behavior closer to GCC.

clang/lib/AST/Decl.cpp

static bool shouldConsiderTemplateVisibility(const FunctionDecl *fn,
                         const FunctionTemplateSpecializationInfo *specInfo) {
   // Include visibility from the template parameters and arguments
   // only if this is not an explicit instantiation or specialization
   // with direct explicit visibility.  (Implicit instantiations won't
   // have a direct attribute.)
   if (!specInfo->isExplicitInstantiationOrSpecialization())
     return true;
 
-  return !fn->hasAttr<VisibilityAttr>();
+  return !fn->hasAttr<VisibilityAttr>() && !specInfo->getTemplate()
+                                                ->getTemplatedDecl()
+                                                ->hasAttr<VisibilityAttr>();
 }
 
 /// Merge in template-related linkage and visibility for the given
 /// function template specialization.
 ///
 /// We don't need a computation kind here because we can assume
 /// LVForValue.
 ///
if (const auto *Var = dyn_cast<VarDecl>(ND)) {
     if (const auto *VTSD = dyn_cast<VarTemplateSpecializationDecl>(Var))
       return getVisibilityOf(VTSD->getSpecializedTemplate()->getTemplatedDecl(),
                              kind);
 
     return std::nullopt;
   }
   // Also handle function template specializations.
   if (const auto *fn = dyn_cast<FunctionDecl>(ND)) {
-    // If the function is a specialization of a template with an
+    // If the function is a specialization of a template,
+    if (FunctionTemplateSpecializationInfo *templateInfo =
+            fn->getTemplateSpecializationInfo()) {
+      // ... If the template has an explicit visibility attribute, use that.
+      if (auto Vis = getVisibilityOf(
+              templateInfo->getTemplate()->getTemplatedDecl(), kind))
+        return Vis;
+      // ... If the template instantiates from a member template with an
       // explicit visibility attribute, use that.
-    if (FunctionTemplateSpecializationInfo *templateInfo
-          = fn->getTemplateSpecializationInfo())
-      return getVisibilityOf(templateInfo->getTemplate()->getTemplatedDecl(),
-                             kind);
+      if (auto *InstantiatedFrom =
+              templateInfo->getTemplate()->getInstantiatedFromMemberTemplate())
+        return getVisibilityOf(InstantiatedFrom->getTemplatedDecl(), kind);
+      return std::nullopt;
+    }
 
     // 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, kind);
 
     return std::nullopt;

clang/test/CodeGenCXX/visibility.cpp

namespace test51 {
 
   struct HIDDEN foo {};
   DEFAULT foo da, db, dc, dd;
   HIDDEN foo ha, hb, hc, hd;
   template<foo *z>
   void DEFAULT zed() {
   }
   template void zed<&da>();
-  // CHECK-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_2daEEEEEvv(
-  // CHECK-HIDDEN-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_2daEEEEEvv(
+  // CHECK-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_2daEEEEEvv(
+  // CHECK-HIDDEN-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_2daEEEEEvv(
 
   template void DEFAULT zed<&db>();
   // CHECK-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_2dbEEEEEvv(
   // CHECK-HIDDEN-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_2dbEEEEEvv(
 
   template void HIDDEN zed<&dc>();
   // CHECK-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_2dcEEEEEvv(
   // CHECK-HIDDEN-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_2dcEEEEEvv(
 
   template void zed<&ha>();
-  // CHECK-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_2haEEEEEvv(
-  // CHECK-HIDDEN-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_2haEEEEEvv(
+  // CHECK-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_2haEEEEEvv(
+  // CHECK-HIDDEN-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_2haEEEEEvv(
 
   template void DEFAULT zed<&hb>();
   // CHECK-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_2hbEEEEEvv(
   // CHECK-HIDDEN-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_2hbEEEEEvv(
 
   template void HIDDEN zed<&hc>();
   // CHECK-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_2hcEEEEEvv(
   // CHECK-HIDDEN-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_2hcEEEEEvv(
struct foo {
     static HIDDEN T zed();
     template <class U> HIDDEN U bar();
   };
   template <class T>
   T foo<T>::zed() { return {}; }
   template <class T> template <class U>
   U foo<T>::bar() { return {}; }
 
+  /// foo<int>::{zed,bar} get the instantiated-from member's HIDDEN, overriding DEFAULT.
   extern template struct DEFAULT foo<int>;
 
   int use() {
     foo<int> o;
     foo<long> p;
     return o.zed() + o.bar<int>() + p.zed() + p.bar<int>();
   }
-  /// FIXME: foo<int>::bar is hidden in GCC w/ or w/o -fvisibility=hidden.
   // CHECK-LABEL: declare hidden noundef i32 @_ZN6test713fooIiE3zedEv(
-  // CHECK-LABEL: define linkonce_odr noundef i32 @_ZN6test713fooIiE3barIiEET_v(
+  // CHECK-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIiE3barIiEET_v(
   // CHECK-LABEL: define linkonce_odr hidden noundef i64 @_ZN6test713fooIlE3zedEv(
-  // CHECK-LABEL: define linkonce_odr noundef i32 @_ZN6test713fooIlE3barIiEET_v(
+  // CHECK-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIlE3barIiEET_v(
   // CHECK-HIDDEN-LABEL: declare hidden noundef i32 @_ZN6test713fooIiE3zedEv(
-  // CHECK-HIDDEN-LABEL: define linkonce_odr noundef i32 @_ZN6test713fooIiE3barIiEET_v(
+  // CHECK-HIDDEN-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIiE3barIiEET_v(
   // CHECK-HIDDEN-LABEL: define linkonce_odr hidden noundef i64 @_ZN6test713fooIlE3zedEv(
   // CHECK-HIDDEN-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIlE3barIiEET_v(
 }