Skip to content

Conversation

@erichkeane
Copy link
Collaborator

Reverts #136128

See discussion here: #136128 (comment)
and : #136128 (comment)

@asavonic can re-submit once the examples provided by @jplehr and/or @DKLoehr are fixed.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 18, 2025
@erichkeane erichkeane requested review from asavonic and jplehr April 18, 2025 15:50
@llvmbot
Copy link
Member

llvmbot commented Apr 18, 2025

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

Changes

Reverts llvm/llvm-project#136128

See discussion here: #136128 (comment)
and : #136128 (comment)

@asavonic can re-submit once the examples provided by @jplehr and/or @DKLoehr are fixed.


Full diff: https://github.com/llvm/llvm-project/pull/136317.diff

2 Files Affected:

  • (modified) clang/lib/AST/Decl.cpp (+3-10)
  • (modified) clang/test/CodeGenCXX/visibility.cpp (+1-37)
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index b59619892979a..ad1cb01592e9b 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -400,9 +400,9 @@ void LinkageComputer::mergeTemplateLV(
   FunctionTemplateDecl *temp = specInfo->getTemplate();
   // Merge information from the template declaration.
   LinkageInfo tempLV = getLVForDecl(temp, computation);
-  // The linkage and visibility of the specialization should be
-  // consistent with the template declaration.
-  LV.mergeMaybeWithVisibility(tempLV, considerVisibility);
+  // The linkage of the specialization should be consistent with the
+  // template declaration.
+  LV.setLinkage(tempLV.getLinkage());
 
   // Merge information from the template parameters.
   LinkageInfo paramsLV =
@@ -1051,13 +1051,6 @@ LinkageComputer::getLVForClassMember(const NamedDecl *D,
     if (const auto *redeclTemp = dyn_cast<RedeclarableTemplateDecl>(temp)) {
       if (isExplicitMemberSpecialization(redeclTemp)) {
         explicitSpecSuppressor = temp->getTemplatedDecl();
-      } else if (const RedeclarableTemplateDecl *from =
-                     redeclTemp->getInstantiatedFromMemberTemplate()) {
-        // If no explicit visibility is specified yet, and this is an
-        // instantiated member of a template, look up visibility there
-        // as well.
-        LinkageInfo fromLV = from->getLinkageAndVisibility();
-        LV.mergeMaybeWithVisibility(fromLV, considerVisibility);
       }
     }
   }
diff --git a/clang/test/CodeGenCXX/visibility.cpp b/clang/test/CodeGenCXX/visibility.cpp
index b69278a71d48e..e1061f3dbd18f 100644
--- a/clang/test/CodeGenCXX/visibility.cpp
+++ b/clang/test/CodeGenCXX/visibility.cpp
@@ -1457,45 +1457,9 @@ namespace test71 {
   // CHECK-LABEL: declare hidden noundef i32 @_ZN6test713fooIiE3zedEv(
   // CHECK-LABEL: define linkonce_odr noundef i32 @_ZN6test713fooIiE3barIiEET_v(
   // CHECK-LABEL: define linkonce_odr hidden noundef i64 @_ZN6test713fooIlE3zedEv(
-  // CHECK-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIlE3barIiEET_v(
+  // CHECK-LABEL: define linkonce_odr 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 i64 @_ZN6test713fooIlE3zedEv(
   // CHECK-HIDDEN-LABEL: define linkonce_odr hidden noundef i32 @_ZN6test713fooIlE3barIiEET_v(
 }
-
-// https://github.com/llvm/llvm-project/issues/103477
-namespace test72 {
-  template <class a>
-  struct t {
-    template <int>
-    static HIDDEN void bar() {}
-  };
-
-  void test() {
-      t<char>::bar<1>();
-  }
-  // CHECK-LABEL: define linkonce_odr hidden void @_ZN6test721tIcE3barILi1EEEvv(
-  // CHECK-HIDDEN-LABEL: define linkonce_odr hidden void @_ZN6test721tIcE3barILi1EEEvv(
-}
-
-// https://github.com/llvm/llvm-project/issues/31462
-namespace test73 {
-  template <class T> struct s {
-    template <class U>
-    __attribute__((__visibility__("hidden"))) U should_not_be_exported();
-  };
-
-  template <class T> template <class U> U s<T>::should_not_be_exported() {
-    return U();
-  }
-
-  extern template struct __attribute__((__visibility__("default"))) s<int>;
-
-  int f() {
-    s<int> o;
-    return o.should_not_be_exported<int>();
-  }
-  // CHECK-LABEL: define linkonce_odr noundef i32 @_ZN6test731sIiE22should_not_be_exportedIiEET_v(
-  // CHECK-HIDDEN-LABEL: define linkonce_odr noundef i32 @_ZN6test731sIiE22should_not_be_exportedIiEET_v(
-}

Copy link
Contributor

@jplehr jplehr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Thanks!

@erichkeane erichkeane merged commit f5947ba into main Apr 18, 2025
14 checks passed
@erichkeane erichkeane deleted the revert-136128-pr103477-visibility-template/patch-1 branch April 18, 2025 17:06
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants