Skip to content
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

[clang][frontend] Support applying the annotate attribute to statements #111841

Merged
merged 8 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
17 changes: 17 additions & 0 deletions clang/include/clang/AST/Attr.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,23 @@ class InheritableParamAttr : public InheritableAttr {
}
};

class InheritableParamOrStmtAttr : public InheritableParamAttr {
protected:
InheritableParamOrStmtAttr(ASTContext &Context,
const AttributeCommonInfo &CommonInfo,
attr::Kind AK, bool IsLateParsed,
bool InheritEvenIfAlreadyPresent)
: InheritableParamAttr(Context, CommonInfo, AK, IsLateParsed,
InheritEvenIfAlreadyPresent) {}

public:
// Implement isa/cast/dyncast/etc.
static bool classof(const Attr *A) {
return A->getKind() >= attr::FirstInheritableParamOrStmtAttr &&
A->getKind() <= attr::LastInheritableParamOrStmtAttr;
}
};

class HLSLAnnotationAttr : public InheritableAttr {
protected:
HLSLAnnotationAttr(ASTContext &Context, const AttributeCommonInfo &CommonInfo,
Expand Down
7 changes: 6 additions & 1 deletion clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,11 @@ class TargetSpecificAttr<TargetSpec target> {
/// redeclarations, even when it's written on a parameter.
class InheritableParamAttr : InheritableAttr;

/// A attribute that is either a declaration attribute or a statement attribute,
/// and if used as a declaration attribute, is inherited by later
/// redeclarations, even when it's written on a parameter.
class InheritableParamOrStmtAttr : InheritableParamAttr;

/// An attribute which changes the ABI rules for a specific parameter.
class ParameterABIAttr : InheritableParamAttr {
let Subjects = SubjectList<[ParmVar]>;
Expand Down Expand Up @@ -928,7 +933,7 @@ def AnalyzerNoReturn : InheritableAttr {
let Documentation = [Undocumented];
}

def Annotate : InheritableParamAttr {
def Annotate : InheritableParamOrStmtAttr {
let Spellings = [Clang<"annotate">];
let Args = [StringArgument<"Annotation">, VariadicExprArgument<"Args">];
// Ensure that the annotate attribute can be used with
Expand Down
20 changes: 20 additions & 0 deletions clang/lib/Sema/SemaStmtAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,24 @@ static Attr *handleNoConvergentAttr(Sema &S, Stmt *St, const ParsedAttr &A,
return ::new (S.Context) NoConvergentAttr(S.Context, A);
}

static Attr *handleAnnotateAttr(Sema &S, Stmt *St, const ParsedAttr &A,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is basically identical to what we do for non-statements, right? We should probably extract this to a 'Check' function that SemaDeclAttr and SemaStmtAttr boht can use.

SourceRange Range) {
// Make sure that there is a string literal as the annotation's first
// argument.
StringRef Str;
if (!S.checkStringLiteralArgumentAttr(A, 0, Str))
return nullptr;

llvm::SmallVector<Expr *, 4> Args;
Args.reserve(A.getNumArgs() - 1);
for (unsigned Idx = 1; Idx < A.getNumArgs(); Idx++) {
assert(!A.isArgIdent(Idx));
Args.push_back(A.getArgAsExpr(Idx));
}

return AnnotateAttr::Create(S.Context, Str, Args.data(), Args.size(), A);
}

template <typename OtherAttr, int DiagIdx>
static bool CheckStmtInlineAttr(Sema &SemaRef, const Stmt *OrigSt,
const Stmt *CurSt,
Expand Down Expand Up @@ -679,6 +697,8 @@ static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const ParsedAttr &A,
return handleMSConstexprAttr(S, St, A, Range);
case ParsedAttr::AT_NoConvergent:
return handleNoConvergentAttr(S, St, A, Range);
case ParsedAttr::AT_Annotate:
return handleAnnotateAttr(S, St, A, Range);
default:
// N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
// declaration attribute is not written on a statement, but this code is
Expand Down
14 changes: 14 additions & 0 deletions clang/lib/Sema/SemaTemplateInstantiate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1537,6 +1537,7 @@ namespace {
NamedDecl *FirstQualifierInScope = nullptr,
bool AllowInjectedClassName = false);

const AnnotateAttr *TransformAnnotateAttr(const AnnotateAttr *AA);
const CXXAssumeAttr *TransformCXXAssumeAttr(const CXXAssumeAttr *AA);
const LoopHintAttr *TransformLoopHintAttr(const LoopHintAttr *LH);
const NoInlineAttr *TransformStmtNoInlineAttr(const Stmt *OrigS,
Expand Down Expand Up @@ -2125,6 +2126,19 @@ TemplateInstantiator::TransformTemplateParmRefExpr(DeclRefExpr *E,
Arg, PackIndex);
}

const AnnotateAttr *
TemplateInstantiator::TransformAnnotateAttr(const AnnotateAttr *AA) {
SmallVector<Expr *> Args;
for (Expr *Arg : AA->args()) {
ExprResult Res = getDerived().TransformExpr(Arg);
if (!Res.isUsable())
return AA;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm... I see this elsewhere, and I'm not thrilled that this is our behavior (returning the uninstantiated version of the attribute).

For Annotate, I wonder if instead we should just continue and NOT add this argument (so if you have one that fails all its args, we continue, but you get an annotate with no args, if 1/2 transform, you get 1/2)?

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know that I like this approach. I agree that silent failure is bad, but silently dropping some arguments seems potentially as bad or worse. I'd rather we produce an error... but if so, I think that might be worth doing separately as part of a cleanup that does so across the board (not just for Annotate). WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We won't be silently dropping them though, TransformExpr will do diagnostics. So we'll just be dropping the ones that are invalid.

Args.push_back(Res.get());
}
return AnnotateAttr::CreateImplicit(getSema().Context, AA->getAnnotation(),
Args.data(), Args.size(), AA->getRange());
}

const CXXAssumeAttr *
TemplateInstantiator::TransformCXXAssumeAttr(const CXXAssumeAttr *AA) {
ExprResult Res = getDerived().TransformExpr(AA->getAssumption());
Expand Down
3 changes: 3 additions & 0 deletions clang/test/AST/attr-print-emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ class C {
ANNOTATE_ATTR int annotated_attr ANNOTATE_ATTR = 0;
// CHECK: __attribute__((annotate("Annotated"))) int annotated_attr __attribute__((annotate("Annotated"))) = 0;

void increment() { [[clang::annotate("Annotated")]] annotated_attr++; }
// CHECK: {{\[\[}}clang::annotate("Annotated")]] annotated_attr++;

// FIXME: We do not print the attribute as written after the type specifier.
int ANNOTATE_ATTR annotated_attr_fixme = 0;
// CHECK: __attribute__((annotate("Annotated"))) int annotated_attr_fixme = 0;
Expand Down
3 changes: 3 additions & 0 deletions clang/test/Sema/annotate.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
void __attribute__((annotate("foo"))) foo(float *a) {
__attribute__((annotate("bar"))) int x;
[[clang::annotate("bar")]] int x2;
[[clang::annotate("bar")]] x2 += 1;
__attribute__((annotate(1))) int y; // expected-error {{expected string literal as argument of 'annotate' attribute}}
[[clang::annotate(1)]] int y2; // expected-error {{expected string literal as argument of 'annotate' attribute}}
__attribute__((annotate("bar", 1))) int z;
[[clang::annotate("bar", 1)]] int z2;
[[clang::annotate("bar", 1)]] z2 += 1;

int u = __builtin_annotation(z, (char*) 0); // expected-error {{second argument to __builtin_annotation must be a non-wide string constant}}
int v = __builtin_annotation(z, (char*) L"bar"); // expected-error {{second argument to __builtin_annotation must be a non-wide string constant}}
Expand All @@ -15,4 +17,5 @@ void __attribute__((annotate("foo"))) foo(float *a) {

__attribute__((annotate())) int c; // expected-error {{'annotate' attribute takes at least 1 argument}}
[[clang::annotate()]] int c2; // expected-error {{'annotate' attribute takes at least 1 argument}}
[[clang::annotate()]] c2 += 1; // expected-error {{'annotate' attribute takes at least 1 argument}}
}
38 changes: 38 additions & 0 deletions clang/test/SemaTemplate/attributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,17 @@ namespace attribute_annotate {
template<typename T> [[clang::annotate("ANNOTATE_FOO"), clang::annotate("ANNOTATE_BAR")]] void HasAnnotations();
void UseAnnotations() { HasAnnotations<int>(); }

// CHECK: FunctionTemplateDecl {{.*}} HasStmtAnnotations
// CHECK: AnnotateAttr {{.*}} "ANNOTATE_BAZ"
// CHECK: FunctionDecl {{.*}} HasStmtAnnotations
// CHECK: TemplateArgument type 'int'
// CHECK: AnnotateAttr {{.*}} "ANNOTATE_BAZ"
template<typename T> void HasStmtAnnotations() {
int x = 0;
[[clang::annotate("ANNOTATE_BAZ")]] x++;
}
void UseStmtAnnotations() { HasStmtAnnotations<int>(); }

// CHECK: FunctionTemplateDecl {{.*}} HasPackAnnotations
// CHECK-NEXT: NonTypeTemplateParmDecl {{.*}} referenced 'int' depth 0 index 0 ... Is
// CHECK-NEXT: FunctionDecl {{.*}} HasPackAnnotations 'void ()'
Expand Down Expand Up @@ -95,6 +106,33 @@ void UseAnnotations() { HasAnnotations<int>(); }
template <int... Is> [[clang::annotate("ANNOTATE_BAZ", Is...)]] void HasPackAnnotations();
void UsePackAnnotations() { HasPackAnnotations<1, 2, 3>(); }

// CHECK: FunctionTemplateDecl {{.*}} HasStmtPackAnnotations
// CHECK-NEXT: NonTypeTemplateParmDecl {{.*}} referenced 'int' depth 0 index 0 ... Is
// CHECK-NEXT: FunctionDecl {{.*}} HasStmtPackAnnotations 'void ()'
// CHECK: AttributedStmt {{.*}}
// CHECK-NEXT: AnnotateAttr {{.*}} "ANNOTATE_QUUX"
// CHECK-NEXT: PackExpansionExpr {{.*}} '<dependent type>'
// CHECK-NEXT: DeclRefExpr {{.*}} 'int' NonTypeTemplateParm {{.*}} 'Is' 'int'
// CHECK: FunctionDecl {{.*}} used HasStmtPackAnnotations 'void ()'
// CHECK-NEXT: TemplateArgument{{.*}} pack
// CHECK-NEXT: TemplateArgument{{.*}} integral '1'
// CHECK-NEXT: TemplateArgument{{.*}} integral '2'
// CHECK-NEXT: TemplateArgument{{.*}} integral '3'
// CHECK: AttributedStmt {{.*}}
// CHECK-NEXT: AnnotateAttr {{.*}} "ANNOTATE_QUUX"
// CHECK-NEXT: PackExpansionExpr {{.*}}
// CHECK-NEXT: SubstNonTypeTemplateParmPackExpr {{.*}}
// CHECK-NEXT: NonTypeTemplateParmDecl {{.*}} referenced 'int' depth 0 index 0 ... Is
// CHECK-NEXT: TemplateArgument pack '<1, 2, 3>'
// CHECK-NEXT: TemplateArgument integral '1'
// CHECK-NEXT: TemplateArgument integral '2'
// CHECK-NEXT: TemplateArgument integral '3'
template <int... Is> void HasStmtPackAnnotations() {
int x = 0;
[[clang::annotate("ANNOTATE_QUUX", Is...)]] x++;
}
void UseStmtPackAnnotations() { HasStmtPackAnnotations<1, 2, 3>(); }

template <int... Is> [[clang::annotate(Is...)]] void HasOnlyPackAnnotation() {} // expected-error {{expected string literal as argument of 'annotate' attribute}}

void UseOnlyPackAnnotations() {
Expand Down
11 changes: 7 additions & 4 deletions clang/utils/TableGen/ClangAttrEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3295,6 +3295,7 @@ static const AttrClassDescriptor AttrClassDescriptors[] = {
{ "INHERITABLE_ATTR", "InheritableAttr" },
{ "DECL_OR_TYPE_ATTR", "DeclOrTypeAttr" },
{ "INHERITABLE_PARAM_ATTR", "InheritableParamAttr" },
{ "INHERITABLE_PARAM_OR_STMT_ATTR", "InheritableParamOrStmtAttr" },
{ "PARAMETER_ABI_ATTR", "ParameterABIAttr" },
{ "HLSL_ANNOTATION_ATTR", "HLSLAnnotationAttr"}
};
Expand Down Expand Up @@ -4328,10 +4329,12 @@ static void GenerateMutualExclusionsChecks(const Record &Attr,

// This means the attribute is either a statement attribute, a decl
// attribute, or both; find out which.
bool CurAttrIsStmtAttr =
Attr.isSubClassOf("StmtAttr") || Attr.isSubClassOf("DeclOrStmtAttr");
bool CurAttrIsDeclAttr =
!CurAttrIsStmtAttr || Attr.isSubClassOf("DeclOrStmtAttr");
bool CurAttrIsStmtAttr = Attr.isSubClassOf("StmtAttr") ||
Attr.isSubClassOf("DeclOrStmtAttr") ||
Attr.isSubClassOf("InheritableParamOrStmtAttr");
bool CurAttrIsDeclAttr = !CurAttrIsStmtAttr ||
Attr.isSubClassOf("DeclOrStmtAttr") ||
Attr.isSubClassOf("InheritableParamOrStmtAttr");

std::vector<std::string> DeclAttrs, StmtAttrs;

Expand Down
Loading