[Clang] Add template argument support for {con,de}structor attributes. #151400
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: None (tynasello) ChangesFixes: #67154 {Con, De}structor attributes in Clang only work with integer priorities (inconsistent with GCC). This commit adds support to these attributes for template arguments. Built off of contributions from abrachet in #67376. Full diff: https://github.com/llvm/llvm-project/pull/151400.diff 7 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 224cb6a32af28..c67aaeda9be70 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1463,9 +1463,13 @@ def ConstInit : InheritableAttr {
def Constructor : InheritableAttr {
let Spellings = [GCC<"constructor">];
- let Args = [DefaultIntArgument<"Priority", 65535>];
+ let Args = [ExprArgument<"Priority", 1>];
let Subjects = SubjectList<[Function]>;
+ let TemplateDependent = 1;
let Documentation = [CtorDtorDocs];
+ let AdditionalMembers = [{
+ static constexpr unsigned int DefaultPriority = 65535;
+ }];
}
def CPUSpecific : InheritableAttr {
@@ -1797,9 +1801,13 @@ def Deprecated : InheritableAttr {
def Destructor : InheritableAttr {
let Spellings = [GCC<"destructor">];
- let Args = [DefaultIntArgument<"Priority", 65535>];
+ let Args = [ExprArgument<"Priority", 1>];
let Subjects = SubjectList<[Function]>;
+ let TemplateDependent = 1;
let Documentation = [CtorDtorDocs];
+ let AdditionalMembers = [{
+ static constexpr unsigned int DefaultPriority = 65535;
+ }];
}
def EmptyBases : InheritableAttr, TargetSpecificAttr<TargetMicrosoftRecordLayout> {
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 834b1c067d84c..1bd6998ca3c75 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -6324,10 +6324,20 @@ void CodeGenModule::EmitGlobalFunctionDefinition(GlobalDecl GD,
SetLLVMFunctionAttributesForDefinition(D, Fn);
+ auto getPriority = [this](auto *Attr) -> int {
+ int priority = Attr->DefaultPriority;
+ Expr *E = Attr->getPriority();
+ if (E) {
+ if (auto CE = E->getIntegerConstantExpr(this->getContext()))
+ priority = CE->getExtValue();
+ }
+ return priority;
+ };
+
if (const ConstructorAttr *CA = D->getAttr<ConstructorAttr>())
- AddGlobalCtor(Fn, CA->getPriority());
+ AddGlobalCtor(Fn, getPriority(CA));
if (const DestructorAttr *DA = D->getAttr<DestructorAttr>())
- AddGlobalDtor(Fn, DA->getPriority(), true);
+ AddGlobalDtor(Fn, getPriority(DA), true);
if (getLangOpts().OpenMP && D->hasAttr<OMPDeclareTargetDeclAttr>())
getOpenMPRuntime().emitDeclareTargetFunction(D, GV);
}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 16b18bcb6a2a0..e03ba4c49e328 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -2152,29 +2152,44 @@ static void handleUnusedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
D->addAttr(::new (S.Context) UnusedAttr(S.Context, AL));
}
+static std::optional<Expr *> sharedGetConstructorDestructorAttrExpr(Sema &S, const ParsedAttr &AL) {
+ Expr *E = nullptr;
+ if (AL.getNumArgs() == 1) {
+ E = AL.getArgAsExpr(0);
+ if (E->isValueDependent()) {
+ if (!E->isTypeDependent() && !E->getType()->isIntegerType()) {
+ S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+ << AL << AANT_ArgumentIntegerConstant << E->getSourceRange();
+ return std::nullopt;
+ }
+ } else {
+ uint32_t priority;
+ if (!S.checkUInt32Argument(AL, AL.getArgAsExpr(0), priority)) {
+ return std::nullopt;
+ }
+ }
+ }
+ return E;
+}
+
static void handleConstructorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
- uint32_t priority = ConstructorAttr::DefaultPriority;
if (S.getLangOpts().HLSL && AL.getNumArgs()) {
S.Diag(AL.getLoc(), diag::err_hlsl_init_priority_unsupported);
return;
}
- if (AL.getNumArgs() &&
- !S.checkUInt32Argument(AL, AL.getArgAsExpr(0), priority))
+ auto E = sharedGetConstructorDestructorAttrExpr(S, AL);
+ if (!E.has_value())
return;
- S.Diag(D->getLocation(), diag::warn_global_constructor)
- << D->getSourceRange();
-
- D->addAttr(::new (S.Context) ConstructorAttr(S.Context, AL, priority));
+ S.Diag(D->getLocation(), diag::warn_global_constructor) << D->getSourceRange();
+ D->addAttr(::new (S.Context) ConstructorAttr(S.Context, AL, E.value()));
}
static void handleDestructorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
- uint32_t priority = DestructorAttr::DefaultPriority;
- if (AL.getNumArgs() &&
- !S.checkUInt32Argument(AL, AL.getArgAsExpr(0), priority))
+ auto E = sharedGetConstructorDestructorAttrExpr(S, AL);
+ if (!E.has_value())
return;
S.Diag(D->getLocation(), diag::warn_global_destructor) << D->getSourceRange();
-
- D->addAttr(::new (S.Context) DestructorAttr(S.Context, AL, priority));
+ D->addAttr(::new (S.Context) DestructorAttr(S.Context, AL, E.value()));
}
template <typename AttrTy>
diff --git a/clang/test/AST/ast-dump-attr.cpp b/clang/test/AST/ast-dump-attr.cpp
index f5a7481571421..a824724abc773 100644
--- a/clang/test/AST/ast-dump-attr.cpp
+++ b/clang/test/AST/ast-dump-attr.cpp
@@ -88,7 +88,8 @@ __attribute__((pointer_with_type_tag(unsigned1,1,2)));
void TestInt(void) __attribute__((constructor(123)));
// CHECK: FunctionDecl{{.*}}TestInt
-// CHECK-NEXT: ConstructorAttr{{.*}} 123
+// CHECK-NEXT: ConstructorAttr
+// CHECK-NEXT: IntegerLiteral{{.*}} 123
static int TestString __attribute__((alias("alias1")));
// CHECK: VarDecl{{.*}}TestString
diff --git a/clang/test/CodeGenCXX/constructor-attr.cpp b/clang/test/CodeGenCXX/constructor-attr.cpp
index ec27ed210ce76..5959d824f78c8 100644
--- a/clang/test/CodeGenCXX/constructor-attr.cpp
+++ b/clang/test/CodeGenCXX/constructor-attr.cpp
@@ -1,6 +1,10 @@
// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
// CHECK: @llvm.global_ctors
+// CHECK-SAME: i32 65535, ptr @_ZN3Foo3fooEv
+// CHECK-SAME: i32 101, ptr @_Z22template_dependent_cxxILi101EEvv
+// CHECK-SAME: i32 102, ptr @_Z22template_dependent_gnuILi102EEvv
+// CHECK-SAME: i32 104, ptr @_Z23template_dependent_nttpIiLi104EEvv
// PR6521
void bar();
@@ -10,3 +14,14 @@ struct Foo {
bar();
}
};
+
+template <int P>
+[[gnu::constructor(P)]] void template_dependent_cxx() {}
+template <int P>
+__attribute__((constructor(P))) void template_dependent_gnu() {}
+template <typename T, int P = sizeof(T) * 26>
+[[gnu::constructor(P)]] void template_dependent_nttp() {}
+
+template void template_dependent_cxx<101>();
+template void template_dependent_gnu<102>();
+template void template_dependent_nttp<int>();
diff --git a/clang/test/CodeGenCXX/destructor-attr.cpp b/clang/test/CodeGenCXX/destructor-attr.cpp
new file mode 100644
index 0000000000000..f0500caed3ccd
--- /dev/null
+++ b/clang/test/CodeGenCXX/destructor-attr.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: @llvm.global_dtors
+// CHECK-SAME: i32 65535, ptr @_ZN3Foo3fooEv
+// CHECK-SAME: i32 101, ptr @_Z22template_dependent_cxxILi101EEvv
+// CHECK-SAME: i32 104, ptr @_Z23template_dependent_nttpIiLi104EEvv
+
+// PR6521
+void bar();
+struct Foo {
+ // CHECK-LABEL: define linkonce_odr {{.*}}void @_ZN3Foo3fooEv
+ static void foo() __attribute__((destructor)) {
+ bar();
+ }
+};
+
+template <int P>
+[[gnu::destructor(P)]] void template_dependent_cxx() {}
+template <typename T, int P = sizeof(T) * 26>
+[[gnu::destructor(P)]] void template_dependent_nttp() {}
+
+template void template_dependent_cxx<101>();
+template void template_dependent_nttp<int>();
diff --git a/clang/test/Sema/constructor-attribute.c b/clang/test/Sema/constructor-attribute.cpp
similarity index 77%
rename from clang/test/Sema/constructor-attribute.c
rename to clang/test/Sema/constructor-attribute.cpp
index 2317c7735bda5..6ebb3d6be8a3f 100644
--- a/clang/test/Sema/constructor-attribute.c
+++ b/clang/test/Sema/constructor-attribute.cpp
@@ -6,11 +6,13 @@ int f(void) __attribute__((constructor(1)));
int f(void) __attribute__((constructor(1,2))); // expected-error {{'constructor' attribute takes no more than 1 argument}}
int f(void) __attribute__((constructor(1.0))); // expected-error {{'constructor' attribute requires an integer constant}}
int f(void) __attribute__((constructor(0x100000000))); // expected-error {{integer constant expression evaluates to value 4294967296 that cannot be represented in a 32-bit unsigned integer type}}
+template <int *I> [[gnu::constructor(I)]] void f(); // expected-error {{'gnu::constructor' attribute requires an integer constant}}
-int x __attribute__((destructor)); // expected-warning {{'destructor' attribute only applies to functions}}
+int y __attribute__((destructor)); // expected-warning {{'destructor' attribute only applies to functions}}
int f(void) __attribute__((destructor));
int f(void) __attribute__((destructor(1)));
int f(void) __attribute__((destructor(1,2))); // expected-error {{'destructor' attribute takes no more than 1 argument}}
int f(void) __attribute__((destructor(1.0))); // expected-error {{'destructor' attribute requires an integer constant}}
+template <int *I> [[gnu::destructor(I)]] void f(); // expected-error {{'gnu::destructor' attribute requires an integer constant}}
void knr() __attribute__((constructor));
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Sirraide
reviewed
Aug 1, 2025
erichkeane
reviewed
Aug 1, 2025
erichkeane
reviewed
Aug 14, 2025
Address PR comments.
tynasello
force-pushed
the
template-args-ctor-dtor-attr
branch
from
ecb3a42 to
d3a1e1f
Compare
|
It seems all comments so far have been addressed here. Re-requesting appropriate reviewers. |
erichkeane
reviewed
Aug 27, 2025
shafik
reviewed
Aug 27, 2025
|
@erichkeane, @shafik, thanks for the comments. I have addressed them, could you please take a look? |
|
I don't have further comments, @erichkeane are you good? |
|
Seems everything is good. Failing test is |
ajordanr-google
approved these changes
Sep 3, 2025
erichkeane
reviewed
Sep 4, 2025
|
1 more change requested, sorry for missing this earlier! |
tynasello
force-pushed
the
template-args-ctor-dtor-attr
branch
from
00b3607 to
49959f8
Compare
erichkeane
reviewed
Sep 5, 2025
tynasello
force-pushed
the
template-args-ctor-dtor-attr
branch
from
49959f8 to
e30002e
Compare
|
@tynasello Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/15766 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #67154
{Con, De}structor attributes in Clang only work with integer priorities (inconsistent with GCC). This commit adds support to these attributes for template arguments.
Built off of contributions from abrachet in #67376.