MaskRay created this revision.
MaskRay added reviewers: clang, aaron.ballman, efriedma, rjmccall, smeenai.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fix https://github.com/llvm/llvm-project/issues/31462
Modify the behavior from 7f90b7d4c29d560b57f6026f886adbf4d7ab4382 (2012) to
clone VisibilityAttr for functions. I think that commit fixed a behavior
but made us more difficult to align with GCC.
The new behavior aligns better with GCC. Specifically, an instantiated
FunctionTemplateDecl/CXXMethodDecl now inherits the original visibility
attribute (test71 in visibility.cpp).
The cloned VisibilityAttr is set to implicit. This is important for
`template void HIDDEN zed<&y>();` to override the original VisibilityAttr
(test51).
For now, it's important not to clone VisibilityAttr for non-FunctionDecl as that
would break test38 (and change test35 in another way to be incompatible
with GCC).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D153835
Files:
clang/include/clang/Basic/Attr.td
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/CodeGenCXX/visibility.cpp
clang/utils/TableGen/ClangAttrEmitter.cpp
Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3627,8 +3627,7 @@
if (!R.getValueAsBit("ASTNode"))
continue;
OS << " case attr::" << R.getName() << ": {\n";
- bool ShouldClone = R.getValueAsBit("Clone") &&
- (!AppliesToDecl ||
+ bool ShouldClone = (!AppliesToDecl ||
R.getValueAsBit("MeaningfulToClassTemplateDefinition"));
if (!ShouldClone) {
Index: clang/test/CodeGenCXX/visibility.cpp
===================================================================
--- clang/test/CodeGenCXX/visibility.cpp
+++ clang/test/CodeGenCXX/visibility.cpp
@@ -951,10 +951,6 @@
}
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 x, y;
@@ -962,8 +958,9 @@
void DEFAULT zed() {
}
template void zed<&x>();
- // CHECK-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_1xEEEEEvv
- // CHECK-HIDDEN-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_1xEEEEEvv
+ /// GCC emits hidden zed<&x> with -fvisibility=hidden.
+ // CHECK-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_1xEEEEEvv
+ // CHECK-HIDDEN-LABEL: define weak_odr void @_ZN6test513zedIXadL_ZNS_1xEEEEEvv
template void HIDDEN zed<&y>();
// CHECK-LABEL: define weak_odr hidden void @_ZN6test513zedIXadL_ZNS_1yEEEEEvv(
@@ -1349,15 +1346,15 @@
template <class T> template <class U>
U foo<T>::bar() { return {}; }
+ /// foo<int>::{zed,bar} instantiate the HIDDEN attributes.
extern template struct DEFAULT foo<int>;
int use() {
foo<int> o;
return o.zed() + o.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-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(
}
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -815,6 +815,18 @@
continue;
}
+ if (auto *A = dyn_cast<VisibilityAttr>(TmplAttr)) {
+ if (isa<FunctionDecl>(Tmpl) && !New->hasAttr<VisibilityAttr>()) {
+ auto *NewA = A->clone(Context);
+ NewA->setImplicit(true);
+ New->addAttr(NewA);
+ }
+ continue;
+ }
+
+ if (auto *A = dyn_cast<TypeVisibilityAttr>(TmplAttr))
+ continue;
+
assert(!TmplAttr->isPackExpansion());
if (TmplAttr->isLateParsed() && LateAttrs) {
// Late parsed attributes must be instantiated and attached after the
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -2870,9 +2870,11 @@
typename T::VisibilityType existingValue = existingAttr->getVisibility();
if (existingValue == value)
return nullptr;
- S.Diag(existingAttr->getLocation(), diag::err_mismatched_visibility);
- S.Diag(CI.getLoc(), diag::note_previous_attribute);
D->dropAttr<T>();
+ if (!existingAttr->isImplicit()) {
+ S.Diag(existingAttr->getLocation(), diag::err_mismatched_visibility);
+ S.Diag(CI.getLoc(), diag::note_previous_attribute);
+ }
}
return ::new (S.Context) T(S.Context, CI, value);
}
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -569,9 +569,6 @@
list<Accessor> Accessors = [];
// Set to true for attributes with arguments which require delayed parsing.
bit LateParsed = 0;
- // Set to false to prevent an attribute from being propagated from a template
- // to the instantiation.
- bit Clone = 1;
// Set to true for attributes which must be instantiated within templates
bit TemplateDependent = 0;
// Set to true for attributes that have a corresponding AST node.
@@ -2993,7 +2990,6 @@
}
def Visibility : InheritableAttr {
- let Clone = 0;
let Spellings = [GCC<"visibility">];
let Args = [EnumArgument<"Visibility", "VisibilityType",
["default", "hidden", "internal", "protected"],
@@ -3003,7 +2999,6 @@
}
def TypeVisibility : InheritableAttr {
- let Clone = 0;
let Spellings = [Clang<"type_visibility">];
let Args = [EnumArgument<"Visibility", "VisibilityType",
["default", "hidden", "internal", "protected"],
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
