[clang][frontend] Support applying the annotate attribute to statements #111841
Conversation
This file contains 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
The underlying code already works; this is just a matter of allowing application of this undocumented attribute.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Eric Astor (ericastor) ChangesBy allowing AnnotateAttr to be applied to statements, users can place arbitrary information in the AST for later use. For example, this can be used for HW-targeted language extensions that involve specialized loop annotations. Full diff: https://github.com/llvm/llvm-project/pull/111841.diff 8 Files Affected:
diff --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h
index ac44e9fdd7c4e9..725498e132fc28 100644
--- a/clang/include/clang/AST/Attr.h
+++ b/clang/include/clang/AST/Attr.h
@@ -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,
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index fbcbf0ed416416..ec3d6e0079f630 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -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]>;
@@ -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
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index b9b3b4063bc383..ee6630bdb429c6 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -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,
+ 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,
@@ -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
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index fd51fa4afcacbf..00a5f81dbca8fe 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -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,
@@ -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;
+ 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());
diff --git a/clang/test/AST/attr-print-emit.cpp b/clang/test/AST/attr-print-emit.cpp
index d8e62ed5f6cd11..a9bca6778d0f1a 100644
--- a/clang/test/AST/attr-print-emit.cpp
+++ b/clang/test/AST/attr-print-emit.cpp
@@ -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;
diff --git a/clang/test/Sema/annotate.c b/clang/test/Sema/annotate.c
index b4551a102e6174..f2ef08d6378975 100644
--- a/clang/test/Sema/annotate.c
+++ b/clang/test/Sema/annotate.c
@@ -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}}
@@ -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}}
}
diff --git a/clang/test/SemaTemplate/attributes.cpp b/clang/test/SemaTemplate/attributes.cpp
index f6c9f13f0842d2..dea19d09745ca2 100644
--- a/clang/test/SemaTemplate/attributes.cpp
+++ b/clang/test/SemaTemplate/attributes.cpp
@@ -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 ()'
@@ -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() {
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 87be48c215e230..cee47fa8a283f2 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -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"}
};
@@ -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;
|
You can test this locally with the following command:git-clang-format --diff 25d9688c43d37c0c918e9b8ab2f67be35b0fb75f 6fd9661bcd2a51a6daa1a2ae9020b7cfdb07e941 --extensions c,h,cpp -- clang/include/clang/AST/Attr.h clang/include/clang/Sema/Sema.h clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/lib/Sema/SemaStmtAttr.cpp clang/lib/Sema/SemaTemplateInstantiate.cpp clang/lib/Sema/SemaTemplateInstantiateDecl.cpp clang/test/AST/attr-print-emit.cpp clang/test/Sema/annotate.c clang/test/SemaTemplate/attributes.cpp clang/utils/TableGen/ClangAttrEmitter.cppView the diff from clang-format here.diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 5e4827ec4a..4890d249c6 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3282,17 +3282,16 @@ namespace {
} // end anonymous namespace
static const AttrClassDescriptor AttrClassDescriptors[] = {
- { "ATTR", "Attr" },
- { "TYPE_ATTR", "TypeAttr" },
- { "STMT_ATTR", "StmtAttr" },
- { "DECL_OR_STMT_ATTR", "DeclOrStmtAttr" },
- { "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"}
-};
+ {"ATTR", "Attr"},
+ {"TYPE_ATTR", "TypeAttr"},
+ {"STMT_ATTR", "StmtAttr"},
+ {"DECL_OR_STMT_ATTR", "DeclOrStmtAttr"},
+ {"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"}};
static void emitDefaultDefine(raw_ostream &OS, StringRef name,
const char *superName) {
|
erichkeane
reviewed
Oct 10, 2024
| for (Expr *Arg : AA->args()) { | ||
| ExprResult Res = getDerived().TransformExpr(Arg); | ||
| if (!Res.isUsable()) | ||
| return AA; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
We won't be silently dropping them though, TransformExpr will do diagnostics. So we'll just be dropping the ones that are invalid.
clang/lib/Sema/SemaStmtAttr.cpp
Outdated
| @@ -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, | |||
There was a problem hiding this comment.
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.
erichkeane
approved these changes
Oct 10, 2024
ericastor
added a commit
to ericastor/llvm-project
that referenced
this pull request
Oct 10, 2024
…ts (llvm#111841) By allowing AnnotateAttr to be applied to statements, users can place arbitrary information in the AST for later use. For example, this can be used for HW-targeted language extensions that involve specialized loop annotations.
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
…ts (llvm#111841) By allowing AnnotateAttr to be applied to statements, users can place arbitrary information in the AST for later use. For example, this can be used for HW-targeted language extensions that involve specialized loop annotations.
bricknerb
pushed a commit
to bricknerb/llvm-project
that referenced
this pull request
Oct 17, 2024
…ts (llvm#111841) By allowing AnnotateAttr to be applied to statements, users can place arbitrary information in the AST for later use. For example, this can be used for HW-targeted language extensions that involve specialized loop annotations.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
By allowing AnnotateAttr to be applied to statements, users can place arbitrary information in the AST for later use.
For example, this can be used for HW-targeted language extensions that involve specialized loop annotations.