llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Fangrui Song (MaskRay)
<details>
<summary>Changes</summary>
When computing the visibility of a template
specialization/instantiation, GCC considers template arguments and
ignores type template parameters.
We currently consider non-type template parameters, which adds a lot of
complexity and is also unnecessary for the majority of cases.
Non-type template arguments that affect the visibility are mostly
`TemplateArgument::Declaration`. If a non-type template argument
involves a `NamedDecl` without a `VisibilityAttr `, the `case
TemplateArgument::Declaration` code in `getLVForTemplateArgumentList`
handles the desired cases.
```
struct HIDDEN foo {};
DEFAULT foo da; // has a VisibilityAttr and is used in a non-type template
argument (uncommon)
foo za; // the majority of cases, no VisibilityAttr
template void zed<&da>(); // w/o the patch: hidden, w/ the patch:
default
template void zed<&za>(); // no behavior change
```
For the updated tests, they all behave the same as GCC.
---
Full diff: https://github.com/llvm/llvm-project/pull/72092.diff
3 Files Affected:
- (modified) clang/lib/AST/Decl.cpp (+9-99)
- (modified) clang/lib/AST/Linkage.h (-3)
- (modified) clang/test/CodeGenCXX/visibility.cpp (+19-35)
``````````diff
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index c5c2edf1bfe3aba..8a84dad4e4452ed 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -244,62 +244,6 @@ LinkageInfo LinkageComputer::getLVForType(const Type &T,
return getTypeLinkageAndVisibility(&T);
}
-/// Get the most restrictive linkage for the types in the given
-/// template parameter list. For visibility purposes, template
-/// parameters are part of the signature of a template.
-LinkageInfo LinkageComputer::getLVForTemplateParameterList(
- const TemplateParameterList *Params, LVComputationKind computation) {
- LinkageInfo LV;
- for (const NamedDecl *P : *Params) {
- // Template type parameters are the most common and never
- // contribute to visibility, pack or not.
- if (isa<TemplateTypeParmDecl>(P))
- continue;
-
- // Non-type template parameters can be restricted by the value type, e.g.
- // template <enum X> class A { ... };
- // We have to be careful here, though, because we can be dealing with
- // dependent types.
- if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(P)) {
- // Handle the non-pack case first.
- if (!NTTP->isExpandedParameterPack()) {
- if (!NTTP->getType()->isDependentType()) {
- LV.merge(getLVForType(*NTTP->getType(), computation));
- }
- continue;
- }
-
- // Look at all the types in an expanded pack.
- for (unsigned i = 0, n = NTTP->getNumExpansionTypes(); i != n; ++i) {
- QualType type = NTTP->getExpansionType(i);
- if (!type->isDependentType())
- LV.merge(getTypeLinkageAndVisibility(type));
- }
- continue;
- }
-
- // Template template parameters can be restricted by their
- // template parameters, recursively.
- const auto *TTP = cast<TemplateTemplateParmDecl>(P);
-
- // Handle the non-pack case first.
- if (!TTP->isExpandedParameterPack()) {
- LV.merge(getLVForTemplateParameterList(TTP->getTemplateParameters(),
- computation));
- continue;
- }
-
- // Look at all expansions in an expanded pack.
- for (unsigned i = 0, n = TTP->getNumExpansionTemplateParameters();
- i != n; ++i) {
- LV.merge(getLVForTemplateParameterList(
- TTP->getExpansionTemplateParameters(i), computation));
- }
- }
-
- return LV;
-}
-
static const Decl *getOutermostFuncOrBlockContext(const Decl *D) {
const Decl *Ret = nullptr;
const DeclContext *DC = D->getDeclContext();
@@ -368,10 +312,9 @@ LinkageComputer::getLVForTemplateArgumentList(const
TemplateArgumentList &TArgs,
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.)
+ // Include visibility from the template 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;
@@ -399,11 +342,6 @@ void LinkageComputer::mergeTemplateLV(
// template declaration.
LV.setLinkage(tempLV.getLinkage());
- // Merge information from the template parameters.
- LinkageInfo paramsLV =
- getLVForTemplateParameterList(temp->getTemplateParameters(),
computation);
- LV.mergeMaybeWithVisibility(paramsLV, considerVisibility);
-
// Merge information from the template arguments.
const TemplateArgumentList &templateArgs = *specInfo->TemplateArguments;
LinkageInfo argsLV = getLVForTemplateArgumentList(templateArgs, computation);
@@ -426,14 +364,13 @@ static bool hasDirectVisibilityAttribute(const NamedDecl
*D,
static bool shouldConsiderTemplateVisibility(
const ClassTemplateSpecializationDecl *spec,
LVComputationKind computation) {
- // Include visibility from the template parameters and arguments
- // only if this is not an explicit instantiation or specialization
- // with direct explicit visibility (and note that implicit
- // instantiations won't have a direct attribute).
+ // Include visibility from the template arguments only if this is not an
+ // explicit instantiation or specialization with direct explicit visibility
+ // (and note that implicit instantiations won't have a direct attribute).
//
- // Furthermore, we want to ignore template parameters and arguments
- // for an explicit specialization when computing the visibility of a
- // member thereof with explicit visibility.
+ // Furthermore, we want to ignore template arguments for an explicit
+ // specialization when computing the visibility of a member thereof with
+ // explicit visibility.
//
// This is a bit complex; let's unpack it.
//
@@ -464,8 +401,6 @@ void LinkageComputer::mergeTemplateLV(
LVComputationKind computation) {
bool considerVisibility = shouldConsiderTemplateVisibility(spec,
computation);
- // Merge information from the template parameters, but ignore
- // visibility if we're only considering template arguments.
ClassTemplateDecl *temp = spec->getSpecializedTemplate();
// Merge information from the template declaration.
LinkageInfo tempLV = getLVForDecl(temp, computation);
@@ -473,11 +408,6 @@ void LinkageComputer::mergeTemplateLV(
// template declaration.
LV.setLinkage(tempLV.getLinkage());
- LinkageInfo paramsLV =
- getLVForTemplateParameterList(temp->getTemplateParameters(), computation);
- LV.mergeMaybeWithVisibility(paramsLV,
- considerVisibility && !hasExplicitVisibilityAlready(computation));
-
// Merge information from the template arguments. We ignore
// template-argument visibility if we've got an explicit
// instantiation with a visibility attribute.
@@ -521,14 +451,6 @@ void LinkageComputer::mergeTemplateLV(LinkageInfo &LV,
LVComputationKind computation) {
bool considerVisibility = shouldConsiderTemplateVisibility(spec,
computation);
- // Merge information from the template parameters, but ignore
- // visibility if we're only considering template arguments.
- VarTemplateDecl *temp = spec->getSpecializedTemplate();
- LinkageInfo tempLV =
- getLVForTemplateParameterList(temp->getTemplateParameters(), computation);
- LV.mergeMaybeWithVisibility(tempLV,
- considerVisibility && !hasExplicitVisibilityAlready(computation));
-
// Merge information from the template arguments. We ignore
// template-argument visibility if we've got an explicit
// instantiation with a visibility attribute.
@@ -858,10 +780,6 @@ LinkageComputer::getLVForNamespaceScopeDecl(const
NamedDecl *D,
// - a template
} else if (const auto *temp = dyn_cast<TemplateDecl>(D)) {
- bool considerVisibility = !hasExplicitVisibilityAlready(computation);
- LinkageInfo tempLV =
- getLVForTemplateParameterList(temp->getTemplateParameters(),
computation);
- LV.mergeMaybeWithVisibility(tempLV, considerVisibility);
// An unnamed namespace or a namespace declared directly or indirectly
// within an unnamed namespace has internal linkage. All other namespaces
@@ -1029,14 +947,6 @@ LinkageComputer::getLVForClassMember(const NamedDecl *D,
// Template members.
} else if (const auto *temp = dyn_cast<TemplateDecl>(D)) {
- bool considerVisibility =
- (!LV.isVisibilityExplicit() &&
- !classLV.isVisibilityExplicit() &&
- !hasExplicitVisibilityAlready(computation));
- LinkageInfo tempLV =
- getLVForTemplateParameterList(temp->getTemplateParameters(),
computation);
- LV.mergeMaybeWithVisibility(tempLV, considerVisibility);
-
if (const auto *redeclTemp = dyn_cast<RedeclarableTemplateDecl>(temp)) {
if (isExplicitMemberSpecialization(redeclTemp)) {
explicitSpecSuppressor = temp->getTemplatedDecl();
diff --git a/clang/lib/AST/Linkage.h b/clang/lib/AST/Linkage.h
index 31f384eb75d0b2e..0907bad8e656dbf 100644
--- a/clang/lib/AST/Linkage.h
+++ b/clang/lib/AST/Linkage.h
@@ -137,9 +137,6 @@ class LinkageComputer {
LinkageInfo getLVForType(const Type &T, LVComputationKind computation);
- LinkageInfo getLVForTemplateParameterList(const TemplateParameterList
*Params,
- LVComputationKind computation);
-
LinkageInfo getLVForValue(const APValue &V, LVComputationKind computation);
public:
diff --git a/clang/test/CodeGenCXX/visibility.cpp
b/clang/test/CodeGenCXX/visibility.cpp
index b017bc8b52d5a5d..30f9a6005b04078 100644
--- a/clang/test/CodeGenCXX/visibility.cpp
+++ b/clang/test/CodeGenCXX/visibility.cpp
@@ -83,9 +83,6 @@ namespace test41 {
}
namespace test48 {
- // Test that we use the visibility of struct foo when instantiating the
- // template. Note that is a case where we disagree with gcc, it produces
- // a default symbol.
struct HIDDEN foo {
};
DEFAULT foo x;
@@ -97,7 +94,7 @@ namespace test48 {
};
bar::zed<&x> y;
- // CHECK: _ZN6test481yE = hidden global
+ // CHECK: _ZN6test481yE = global
// CHECK-HIDDEN: _ZN6test481yE = hidden global
}
@@ -137,22 +134,22 @@ namespace test73 {
template int HIDDEN var<&hc>;
int use() { return var<&dd> + var<&hd>; }
- // CHECK: @_ZN6test733varIXadL_ZNS_2daEEEEE = weak_odr hidden global
i32 0
+ // CHECK: @_ZN6test733varIXadL_ZNS_2daEEEEE = weak_odr global i32 0
// CHECK-NEXT: @_ZN6test733varIXadL_ZNS_2dbEEEEE = weak_odr global i32 0
// CHECK-NEXT: @_ZN6test733varIXadL_ZNS_2dcEEEEE = weak_odr hidden global
i32 0
// CHECK-NEXT: @_ZN6test733varIXadL_ZNS_2haEEEEE = weak_odr hidden global
i32 0
// CHECK-NEXT: @_ZN6test733varIXadL_ZNS_2hbEEEEE = weak_odr global i32 0
// CHECK-NEXT: @_ZN6test733varIXadL_ZNS_2hcEEEEE = weak_odr hidden global
i32 0
- // CHECK: @_ZN6test733varIXadL_ZNS_2ddEEEEE = linkonce_odr hidden
global i32 0
+ // CHECK: @_ZN6test733varIXadL_ZNS_2ddEEEEE = linkonce_odr global i32 0
// CHECK-NEXT: @_ZN6test733varIXadL_ZNS_2hdEEEEE = linkonce_odr hidden
global i32 0
- // CHECK-HIDDEN: @_ZN6test733varIXadL_ZNS_2daEEEEE = weak_odr hidden
global i32 0
+ // CHECK-HIDDEN: @_ZN6test733varIXadL_ZNS_2daEEEEE = weak_odr global
i32 0
// CHECK-HIDDEN-NEXT: @_ZN6test733varIXadL_ZNS_2dbEEEEE = weak_odr global
i32 0
// CHECK-HIDDEN-NEXT: @_ZN6test733varIXadL_ZNS_2dcEEEEE = weak_odr hidden
global i32 0
// CHECK-HIDDEN-NEXT: @_ZN6test733varIXadL_ZNS_2haEEEEE = weak_odr hidden
global i32 0
// CHECK-HIDDEN-NEXT: @_ZN6test733varIXadL_ZNS_2hbEEEEE = weak_odr global
i32 0
// CHECK-HIDDEN-NEXT: @_ZN6test733varIXadL_ZNS_2hcEEEEE = weak_odr hidden
global i32 0
- // CHECK-HIDDEN: @_ZN6test733varIXadL_ZNS_2ddEEEEE = linkonce_odr
hidden global i32 0
+ // CHECK-HIDDEN: @_ZN6test733varIXadL_ZNS_2ddEEEEE = linkonce_odr
global i32 0
// CHECK-HIDDEN-NEXT: @_ZN6test733varIXadL_ZNS_2hdEEEEE = linkonce_odr
hidden global i32 0
}
@@ -963,10 +960,9 @@ namespace test47 {
}
namespace test49 {
- // Test that we use the visibility of struct foo when instantiating the
- // template. Note that is a case where we disagree with gcc, it produces
- // a default symbol.
-
+ // When instantiating the template, we constraint the visibility with the
+ // non-type template argument &x. The type of &x does not constraint the
+ // visibility.
struct HIDDEN foo {
};
@@ -979,15 +975,11 @@ namespace test49 {
};
template void bar::zed<&x>();
- // CHECK-LABEL: define weak_odr hidden void
@_ZN6test493bar3zedIXadL_ZNS_1xEEEEEvv
+ // CHECK-LABEL: define weak_odr void @_ZN6test493bar3zedIXadL_ZNS_1xEEEEEvv
// CHECK-HIDDEN-LABEL: define weak_odr hidden void
@_ZN6test493bar3zedIXadL_ZNS_1xEEEEEvv
}
namespace test50 {
- // Test that we use the visibility of struct foo when instantiating the
- // template. Note that is a case where we disagree with gcc, it produces
- // a default symbol.
-
struct HIDDEN foo {
};
DEFAULT foo x;
@@ -997,15 +989,11 @@ namespace test50 {
}
};
template void bar<&x>::zed();
- // CHECK-LABEL: define weak_odr hidden void
@_ZN6test503barIXadL_ZNS_1xEEEE3zedEv
- // CHECK-HIDDEN-LABEL: define weak_odr hidden void
@_ZN6test503barIXadL_ZNS_1xEEEE3zedEv
+ // CHECK-LABEL: define weak_odr void @_ZN6test503barIXadL_ZNS_1xEEEE3zedEv
+ // CHECK-HIDDEN-LABEL: define weak_odr void
@_ZN6test503barIXadL_ZNS_1xEEEE3zedEv
}
namespace test51 {
- // Test that we use the visibility of struct foo when instantiating the
- // template. Note that is a case where we disagree with gcc, it produces
- // a default symbol.
-
struct HIDDEN foo {};
DEFAULT foo da, db, dc, dd, de, df;
HIDDEN foo ha, hb, hc, hd, he, hf;
@@ -1013,8 +1001,8 @@ namespace test51 {
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(
@@ -1041,10 +1029,10 @@ namespace test51 {
template void zed<&hd>();
template void DEFAULT zed<&he>();
#pragma GCC visibility pop
- // CHECK-LABEL: define weak_odr hidden void
@_ZN6test513zedIXadL_ZNS_2ddEEEEEvv(
+ // CHECK-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_2ddEEEEEvv(
// CHECK-LABEL: define weak_odr hidden void
@_ZN6test513zedIXadL_ZNS_2hdEEEEEvv(
// CHECK-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_2heEEEEEvv(
- // CHECK-HIDDEN-LABEL: define weak_odr hidden void
@_ZN6test513zedIXadL_ZNS_2ddEEEEEvv(
+ // CHECK-HIDDEN-LABEL: define weak_odr void
@_ZN6test513zedIXadL_ZNS_2ddEEEEEvv(
// CHECK-HIDDEN-LABEL: define weak_odr hidden void
@_ZN6test513zedIXadL_ZNS_2hdEEEEEvv(
// CHECK-HIDDEN-LABEL: define weak_odr void
@_ZN6test513zedIXadL_ZNS_2heEEEEEvv(
@@ -1052,17 +1040,13 @@ namespace test51 {
zed<&df>();
zed<&hf>();
}
- // CHECK-LABEL: define linkonce_odr hidden void
@_ZN6test513zedIXadL_ZNS_2dfEEEEEvv(
+ // CHECK-LABEL: define linkonce_odr void @_ZN6test513zedIXadL_ZNS_2dfEEEEEvv(
// CHECK-LABEL: define linkonce_odr hidden void
@_ZN6test513zedIXadL_ZNS_2hfEEEEEvv(
- // CHECK-HIDDEN-LABEL: define linkonce_odr hidden void
@_ZN6test513zedIXadL_ZNS_2dfEEEEEvv(
+ // CHECK-HIDDEN-LABEL: define linkonce_odr void
@_ZN6test513zedIXadL_ZNS_2dfEEEEEvv(
// CHECK-HIDDEN-LABEL: define linkonce_odr hidden void
@_ZN6test513zedIXadL_ZNS_2hfEEEEEvv(
}
namespace test52 {
- // Test that we use the linkage of struct foo when instantiating the
- // template. Note that is a case where we disagree with gcc, it produces
- // an external symbol.
-
namespace {
struct foo {
};
@@ -1273,8 +1257,8 @@ namespace test63 {
A::foo<E0>();
A::B<E0>::foo();
}
- // CHECK-LABEL: define linkonce_odr hidden void
@_ZN6test631A3fooILNS_1EE0EEEvv()
- // CHECK-LABEL: define linkonce_odr hidden void
@_ZN6test631A1BILNS_1EE0EE3fooEv()
+ // CHECK-LABEL: define linkonce_odr void @_ZN6test631A3fooILNS_1EE0EEEvv()
+ // CHECK-LABEL: define linkonce_odr void @_ZN6test631A1BILNS_1EE0EE3fooEv()
}
// Don't ignore the visibility of template arguments just because we
``````````
</details>
https://github.com/llvm/llvm-project/pull/72092
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
