Skip to content

[clang] All {con,de}structor attributes to use template args #67376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -1156,9 +1156,11 @@ 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 Documentation = [CtorDtorDocs];
let TemplateDependent = 1;
let AdditionalMembers = [{ static const int DefaultPriority = 65535; }];
Copy link
Collaborator

Choose a reason for hiding this comment

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

@AaronBallman : I see in your other patch (#67360) you're using this as a magic number! Might be useful to get on the same page as this author.

Suggested change
let AdditionalMembers = [{ static const int DefaultPriority = 65535; }];
let AdditionalMembers = [{ static constexpr int DefaultPriority = 65535; }];

}

def CPUSpecific : InheritableAttr {
Expand Down Expand Up @@ -1422,9 +1424,11 @@ 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 Documentation = [CtorDtorDocs];
let TemplateDependent = 1;
let AdditionalMembers = [{ static const int DefaultPriority = 65535; }];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let AdditionalMembers = [{ static const int DefaultPriority = 65535; }];
let AdditionalMembers = [{ static constexpr int DefaultPriority = 65535; }];

Also wonder if these should be unsigned?

}

def EmptyBases : InheritableAttr, TargetSpecificAttr<TargetMicrosoftCXXABI> {
Expand Down
17 changes: 14 additions & 3 deletions clang/lib/CodeGen/CodeGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
#include "clang/AST/Mangle.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/StmtVisitor.h"
Expand Down Expand Up @@ -5661,10 +5662,20 @@ void CodeGenModule::EmitGlobalFunctionDefinition(GlobalDecl GD,
setNonAliasAttributes(GD, Fn);
SetLLVMFunctionAttributesForDefinition(D, Fn);

if (const ConstructorAttr *CA = D->getAttr<ConstructorAttr>())
AddGlobalCtor(Fn, CA->getPriority());
auto getPrio = [this](auto *Attr) -> int {
Expr *E = Attr->getPriority();
if (!E)
return Attr->DefaultPriority;
if (auto CE = E->getIntegerConstantExpr(getContext()))
return CE->getExtValue();
return Attr->DefaultPriority;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This condition seems to me should be an assertion, right?

};

if (const ConstructorAttr *CA = D->getAttr<ConstructorAttr>()) {
AddGlobalCtor(Fn, getPrio(CA));
}
if (const DestructorAttr *DA = D->getAttr<DestructorAttr>())
AddGlobalDtor(Fn, DA->getPriority(), true);
AddGlobalDtor(Fn, getPrio(DA), true);
if (D->hasAttr<AnnotateAttr>())
AddGlobalAnnotations(D, Fn);
if (getLangOpts().OpenMP && D->hasAttr<OMPDeclareTargetDeclAttr>())
Expand Down
33 changes: 22 additions & 11 deletions clang/lib/Sema/SemaDeclAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2352,26 +2352,37 @@ static void handleUnusedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
D->addAttr(::new (S.Context) UnusedAttr(S.Context, AL));
}

template <typename Attr>
static void handleCtorDtorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
uint32_t priority = Attr::DefaultPriority;
Expr *E = nullptr;
if (AL.getNumArgs()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We also should ensure we don't have more than 1 argument.

E = AL.getArgAsExpr(0);
if (E->isValueDependent()) {
if (!E->isTypeDependent() && !E->getType()->isIntegerType()) {
S.Diag(getAttrLoc(AL), diag::err_attribute_argument_type)
<< &AL << AANT_ArgumentIntegerConstant << E->getSourceRange();
return;
}
} else if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), priority)) {
return;
}
}

D->addAttr(::new (S.Context) Attr(S.Context, AL, 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() &&
!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), priority))
return;

D->addAttr(::new (S.Context) ConstructorAttr(S.Context, AL, priority));
handleCtorDtorAttr<ConstructorAttr>(S, D, AL);
}

static void handleDestructorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
uint32_t priority = DestructorAttr::DefaultPriority;
if (AL.getNumArgs() &&
!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), priority))
return;

D->addAttr(::new (S.Context) DestructorAttr(S.Context, AL, priority));
return handleCtorDtorAttr<DestructorAttr>(S, D, AL);
}

template <typename AttrTy>
Expand Down
14 changes: 14 additions & 0 deletions clang/test/CodeGenCXX/constructor-attr.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s

// CHECK: @llvm.global_ctors
// CHECK-SAME: i32 101, ptr @_Z18template_dependentILi101EEvv
// CHECK-SAME: i32 108, ptr @_Z18template_dependentILi108EEvv
// CHECK-SAME: i32 111, ptr @_Z19template_dependent2ILi111EEvv

// PR6521
void bar();
Expand All @@ -10,3 +13,14 @@ struct Foo {
bar();
}
};

template <int P>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please write a test where the argument is NTTP dependent on another template argument.

[[gnu::constructor(P)]] void template_dependent() {}

template void template_dependent<101>();
template void template_dependent<100 + 8>();

template <int P>
__attribute__((constructor(P))) void template_dependent2() {}

template void template_dependent2<111>();
20 changes: 20 additions & 0 deletions clang/test/CodeGenCXX/destructor-attr.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s

// CHECK: @llvm.global_dtors
// CHECK-SAME: i32 101, ptr @_Z18template_dependentILi101EEvv
// CHECK-SAME: i32 108, ptr @_Z18template_dependentILi108EEvv

// PR6521
void bar();
struct Foo {
// CHECK-LABEL: define linkonce_odr {{.*}}void @_ZN3Foo3fooEv
static void foo() __attribute__((constructor)) {
bar();
}
};

template <int P>
[[gnu::destructor(P)]] void template_dependent() {}

template void template_dependent<101>();
template void template_dependent<100 + 8>();
13 changes: 13 additions & 0 deletions clang/test/Sema/ctor-dtor-attr.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wno-strict-prototypes %s

template <int I> [[gnu::constructor(I)]] void ok_ctor();
template <int I> __attribute__((constructor(I))) void ok2_ctor();

template <int *I> [[gnu::constructor(I)]] void bad_ctor(); // expected-error {{'constructor' attribute requires an integer constant}}
template <int *I> __attribute__((constructor(I))) void bad2_ctor(); // expected-error {{'constructor' attribute requires an integer constant}}

template <int I> [[gnu::destructor(I)]] void ok_ctor();
template <int I> __attribute__((destructor(I))) void ok2_dtor();

template <int *I> [[gnu::destructor(I)]] void bad_dtor(); // expected-error {{'destructor' attribute requires an integer constant}}
template <int *I> __attribute__((destructor(I))) void bad2_dtor(); // expected-error {{'destructor' attribute requires an integer constant}}