[clang][frontend] Add support for attribute plugins for statement attributes #110334
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
…ributes We already have support for declaration attributes; this is just a matter of extending the plugin infrastructure to cover one more case.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Eric Astor (ericastor) ChangesWe already have support for declaration attributes; this is just a matter of extending the plugin infrastructure to cover one more case. Full diff: https://github.com/llvm/llvm-project/pull/110334.diff 5 Files Affected:
diff --git a/clang/docs/ClangPlugins.rst b/clang/docs/ClangPlugins.rst
index 001e66e434efb1..92e41fb5877fe8 100644
--- a/clang/docs/ClangPlugins.rst
+++ b/clang/docs/ClangPlugins.rst
@@ -92,11 +92,6 @@ The members of ``ParsedAttrInfo`` that a plugin attribute must define are:
attribute, each of which consists of an attribute syntax and how the
attribute name is spelled for that syntax. If the syntax allows a scope then
the spelling must be "scope::attr" if a scope is present or "::attr" if not.
- * ``handleDeclAttribute``, which is the function that applies the attribute to
- a declaration. It is responsible for checking that the attribute's arguments
- are valid, and typically applies the attribute by adding an ``Attr`` to the
- ``Decl``. It returns either ``AttributeApplied``, to indicate that the
- attribute was successfully applied, or ``AttributeNotApplied`` if it wasn't.
The members of ``ParsedAttrInfo`` that may need to be defined, depending on the
attribute, are:
@@ -105,6 +100,18 @@ attribute, are:
arguments to the attribute.
* ``diagAppertainsToDecl``, which checks if the attribute has been used on the
right kind of declaration and issues a diagnostic if not.
+ * ``handleDeclAttribute``, which is the function that applies the attribute to
+ a declaration. It is responsible for checking that the attribute's arguments
+ are valid, and typically applies the attribute by adding an ``Attr`` to the
+ ``Decl``. It returns either ``AttributeApplied``, to indicate that the
+ attribute was successfully applied, or ``AttributeNotApplied`` if it wasn't.
+ * ``diagAppertainsToStmt``, which checks if the attribute has been used on the
+ right kind of statement and issues a diagnostic if not.
+ * ``handleStmtAttribute``, which is the function that applies the attribute to
+ a statement. It is responsible for checking that the attribute's arguments
+ are valid, and typically applies the attribute by adding an ``Attr`` to the
+ ``Stmt``. It returns either ``AttributeApplied``, to indicate that the
+ attribute was successfully applied, or ``AttributeNotApplied`` if it wasn't.
* ``diagLangOpts``, which checks if the attribute is permitted for the current
language mode and issues a diagnostic if not.
* ``existsInTarget``, which checks if the attribute is permitted for the given
diff --git a/clang/examples/Attribute/Attribute.cpp b/clang/examples/Attribute/Attribute.cpp
index 9d6cf9ae36c6a6..07dd19321195c8 100644
--- a/clang/examples/Attribute/Attribute.cpp
+++ b/clang/examples/Attribute/Attribute.cpp
@@ -94,6 +94,55 @@ struct ExampleAttrInfo : public ParsedAttrInfo {
}
return AttributeApplied;
}
+
+ bool diagAppertainsToStmt(Sema &S, const ParsedAttr &Attr,
+ const Stmt *St) const override {
+ // This attribute appertains to for loop statements only.
+ if (!isa<ForStmt>(St)) {
+ S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type_str)
+ << Attr << Attr.isRegularKeywordAttribute() << "for loop statements";
+ return false;
+ }
+ return true;
+ }
+
+ AttrHandling handleStmtAttribute(Sema &S, Stmt *St, const ParsedAttr &Attr,
+ class Attr *&Result) const override {
+ // We make some rules here:
+ // 1. Only accept at most 3 arguments here.
+ // 2. The first argument must be a string literal if it exists.
+ if (Attr.getNumArgs() > 3) {
+ unsigned ID = S.getDiagnostics().getCustomDiagID(
+ DiagnosticsEngine::Error,
+ "'example' attribute only accepts at most three arguments");
+ S.Diag(Attr.getLoc(), ID);
+ return AttributeNotApplied;
+ }
+ // If there are arguments, the first argument should be a string literal.
+ if (Attr.getNumArgs() > 0) {
+ auto *Arg0 = Attr.getArgAsExpr(0);
+ StringLiteral *Literal =
+ dyn_cast<StringLiteral>(Arg0->IgnoreParenCasts());
+ if (!Literal) {
+ unsigned ID = S.getDiagnostics().getCustomDiagID(
+ DiagnosticsEngine::Error, "first argument to the 'example' "
+ "attribute must be a string literal");
+ S.Diag(Attr.getLoc(), ID);
+ return AttributeNotApplied;
+ }
+ SmallVector<Expr *, 16> ArgsBuf;
+ for (unsigned i = 0; i < Attr.getNumArgs(); i++) {
+ ArgsBuf.push_back(Attr.getArgAsExpr(i));
+ }
+ Result = AnnotateAttr::Create(S.Context, "example", ArgsBuf.data(),
+ ArgsBuf.size(), Attr.getRange());
+ } else {
+ // Attach an annotate attribute to the Decl.
+ Result = AnnotateAttr::Create(S.Context, "example", nullptr, 0,
+ Attr.getRange());
+ }
+ return AttributeApplied;
+ }
};
} // namespace
diff --git a/clang/include/clang/Basic/ParsedAttrInfo.h b/clang/include/clang/Basic/ParsedAttrInfo.h
index 537d8f3391d589..fab5c6f1377d27 100644
--- a/clang/include/clang/Basic/ParsedAttrInfo.h
+++ b/clang/include/clang/Basic/ParsedAttrInfo.h
@@ -24,6 +24,7 @@
namespace clang {
+class Attr;
class Decl;
class LangOptions;
class ParsedAttr;
@@ -154,6 +155,15 @@ struct ParsedAttrInfo {
const ParsedAttr &Attr) const {
return NotHandled;
}
+ /// If this ParsedAttrInfo knows how to handle this ParsedAttr applied to this
+ /// Stmt then do so (referencing the resulting Attr in Result) and return
+ /// either AttributeApplied if it was applied or AttributeNotApplied if it
+ /// wasn't. Otherwise return NotHandled.
+ virtual AttrHandling handleStmtAttribute(Sema &S, Stmt *St,
+ const ParsedAttr &Attr,
+ class Attr *&Result) const {
+ return NotHandled;
+ }
static const ParsedAttrInfo &get(const AttributeCommonInfo &A);
static ArrayRef<const ParsedAttrInfo *> getAllBuiltin();
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index b9b3b4063bc383..3ebd1148e8bbfb 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -680,6 +680,10 @@ static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const ParsedAttr &A,
case ParsedAttr::AT_NoConvergent:
return handleNoConvergentAttr(S, St, A, Range);
default:
+ if (Attr *AT = nullptr; A.getInfo().handleStmtAttribute(S, St, A, AT) !=
+ ParsedAttrInfo::NotHandled) {
+ return AT;
+ }
// N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
// declaration attribute is not written on a statement, but this code is
// needed for attributes in Attr.td that do not list any subjects.
diff --git a/clang/test/Frontend/plugin-attribute.cpp b/clang/test/Frontend/plugin-attribute.cpp
index 1c5a2440b26888..2e9d171a0095a9 100644
--- a/clang/test/Frontend/plugin-attribute.cpp
+++ b/clang/test/Frontend/plugin-attribute.cpp
@@ -4,11 +4,33 @@
// REQUIRES: plugins, examples
//--- good_attr.cpp
// expected-no-diagnostics
-void fn1a() __attribute__((example)) {}
-[[example]] void fn1b() {}
-[[plugin::example]] void fn1c() {}
-void fn2() __attribute__((example("somestring", 1, 2.0))) {}
-// CHECK-COUNT-4: -AnnotateAttr 0x{{[0-9a-z]+}} {{<col:[0-9]+(, col:[0-9]+)?>}} "example"
+void fn1a() __attribute__((example)) {
+ __attribute__((example)) for (int i = 0; i < 10; ++i) {}
+}
+[[example]] void fn1b() {
+ [[example]] for (int i = 0; i < 10; ++i) {}
+}
+[[plugin::example]] void fn1c() {
+ [[plugin::example]] for (int i = 0; i < 10; ++i) {}
+}
+void fn2() __attribute__((example("somestring", 1, 2.0))) {
+ __attribute__((example("abc", 3, 4.0))) for (int i = 0; i < 10; ++i) {}
+}
+// CHECK: -AttributedStmt 0x{{[0-9a-z]+}} {{<line:[0-9]+:[0-9]+(, col:[0-9]+)?>}}
+// CHECK: -AnnotateAttr 0x{{[0-9a-z]+}} {{<col:[0-9]+(, col:[0-9]+)?>}} "example"
+// CHECK: -AnnotateAttr 0x{{[0-9a-z]+}} {{<line:[0-9]+:[0-9]+(, col:[0-9]+)?>}} "example"
+// CHECK: -AttributedStmt 0x{{[0-9a-z]+}} {{<line:[0-9]+:[0-9]+(, col:[0-9]+)?>}}
+// CHECK: -AnnotateAttr 0x{{[0-9a-z]+}} {{<col:[0-9]+(, col:[0-9]+)?>}} "example"
+// CHECK: -AnnotateAttr 0x{{[0-9a-z]+}} {{<line:[0-9]+:[0-9]+(, col:[0-9]+)?>}} "example"
+// CHECK: -AttributedStmt 0x{{[0-9a-z]+}} {{<line:[0-9]+:[0-9]+(, col:[0-9]+)?>}}
+// CHECK: -AnnotateAttr 0x{{[0-9a-z]+}} {{<col:[0-9]+(, col:[0-9]+)?>}} "example"
+// CHECK: -AnnotateAttr 0x{{[0-9a-z]+}} {{<line:[0-9]+:[0-9]+(, col:[0-9]+)?>}} "example"
+// CHECK: -AttributedStmt 0x{{[0-9a-z]+}} {{<line:[0-9]+:[0-9]+(, col:[0-9]+)?>}}
+// CHECK: -AnnotateAttr 0x{{[0-9a-z]+}} {{<col:[0-9]+(, col:[0-9]+)?>}} "example"
+// CHECK: -StringLiteral 0x{{[0-9a-z]+}} {{<col:[0-9]+(, col:[0-9]+)?>}} 'const char[{{[0-9]+}}]' lvalue "abc"
+// CHECK: -IntegerLiteral 0x{{[0-9a-z]+}} {{<col:[0-9]+(, col:[0-9]+)?>}} 'int' 3
+// CHECK: -FloatingLiteral 0x{{[0-9a-z]+}} {{<col:[0-9]+(, col:[0-9]+)?>}} 'double' 4.000000e+00
+// CHECK: -AnnotateAttr 0x{{[0-9a-z]+}} {{<line:[0-9]+:[0-9]+(, col:[0-9]+)?>}} "example"
// CHECK: -StringLiteral 0x{{[0-9a-z]+}} {{<col:[0-9]+(, col:[0-9]+)?>}} 'const char[{{[0-9]+}}]' lvalue "somestring"
// CHECK: -IntegerLiteral 0x{{[0-9a-z]+}} {{<col:[0-9]+(, col:[0-9]+)?>}} 'int' 1
// CHECK: -FloatingLiteral 0x{{[0-9a-z]+}} {{<col:[0-9]+(, col:[0-9]+)?>}} 'double' 2.000000e+00
@@ -18,5 +40,9 @@ int var1 __attribute__((example("otherstring"))) = 1; // expected-warning {{'exa
class Example {
void __attribute__((example)) fn3(); // expected-error {{'example' attribute only allowed at file scope}}
};
-void fn4() __attribute__((example(123))) { } // expected-error {{first argument to the 'example' attribute must be a string literal}}
-void fn5() __attribute__((example("a","b", 3, 4.0))) { } // expected-error {{'example' attribute only accepts at most three arguments}}
+void fn4() __attribute__((example(123))) { // expected-error {{first argument to the 'example' attribute must be a string literal}}
+ __attribute__((example("somestring"))) while (true); // expected-warning {{'example' attribute only applies to for loop statements}}
+}
+void fn5() __attribute__((example("a","b", 3, 4.0))) { // expected-error {{'example' attribute only accepts at most three arguments}}
+ __attribute__((example("a","b", 3, 4.0))) for (int i = 0; i < 10; ++i) {} // expected-error {{'example' attribute only accepts at most three arguments}}
+}
|
|
This looks like a relatively straightforward extension in line with the current Clang interfaces. Therefore, I feel we should accept this. However, please wait for feedback from Clang owners to make sure this does not go against some non-obvious trade-offs or future plans that Clang has. |
Sirraide
reviewed
Oct 2, 2024
There was a problem hiding this comment.
Looks ok to me, but I’ll leave it to @erichkeane to approve this as he’s the attributes code owner.
erichkeane
reviewed
Oct 2, 2024
There was a problem hiding this comment.
So the problem with statement attributes is that we don't really have a good way to do instantiation of them on a template, which is why we held off on this in the first place. The infrastructure for instantiation DOES happen on decl attributes automatically anyway, so that made it easier to do.
So please ensure there is some sort of hooks in the statement attributes to ensure that they get a chance to instantatiate themselves (and add tests with templates in them!).
This is a fair point... how are standard statement attributes handled inside of templates? (Assuming clang supports that.) |
All our handling of attributes is pretty manual for anything with arguments. We have a bit of an automatic handling for many attributes, but arguments aren't. I think there is a section in |
|
So... I think this turned out to be surprisingly easy. After all, we don't actually allow custom attributes to create entirely new attributes in the AST, since they can't create new legal AST entries... instead, the example creates AnnotateAttrs that can then be observed & acted on by the compiler-extending plugin. It looks like reaching the same level of support just required adding AnnotateAttr support to the template instantiation logic. Am I missing anything else? @erichkeane |
erichkeane
reviewed
Oct 9, 2024
There was a problem hiding this comment.
This seems reasonable? I actually didn't know we weren't letting them create new attributes, and were just using AnnotateAttr in the AST. I think @AaronBallman should do 1 run through this as he understands the plugins better than I, but this otherwise LGTM.
| return AA; | ||
| Args.push_back(Res.get()); | ||
| } | ||
| return AnnotateAttr::CreateImplicit(getSema().Context, AA->getAnnotation(), |
There was a problem hiding this comment.
we probably want to transform the annotation as well? or does StringArgument not work for dependent values?
There was a problem hiding this comment.
I think StringArgument only handles StringRefs? I don't see a way it could be given a dependent value.
…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.
…1214) In tosa valiation pass, change the type of profile option to ListOption. Now TOSA profiles is turned from hierarchical to composable. Each profile is an independent set, i.e. an target can implement multiple profiles. Set the profile option to none by default, and limit to profiles if requested. The profiles can be specified via command line, e.g. $ mlir-opt ... --tosa-validate="profile=bi,mi" which tells the valiation pass that BI and MI are enabled. Change-Id: I1fb8d0c1b27eccd768349b6eb4234093313efb57
LLVM now triggers an assertion when the format string and arguments don't match. Fix a variety of incorrect format strings I discovered when enabling logging with a debug build.
…dPtrOrigin. (llvm#111222) Ignore std::forward when it appears while looking for the pointer origin.
…XXInheritedCtorInitExpr. (llvm#111198)
Make isUncountedPtr take QualType as an argument instead of Type*. This simplifies some code.
A COMMON block is a named area of memory that holds a collection of variables. Fortran subprograms may map the COMMON block memory area to a list of variables. A common block is represented in LLVM debug by DICommonBlock. This PR adds support for this in MLIR. The changes are mostly mechanical apart from small change to access the DICompileUnit when the scope of the variable is DICommonBlock. --------- Co-authored-by: Tobias Gysi <[email protected]>
This is a purely mechanical commit for fixing the indentation of the runtimes' CMakeLists files after llvm#80007. That PR didn't update the indentation in order to make the diff easier to review and for merge conflicts to be easier to resolve (for downstream changes). This doesn't change any code, it only reindents it.
In [[NVPTX] Improve lowering of v4i8](llvm@cbafb6f) @Artem-B add the ability to lower ISD::BUILD_VECTOR with bfi PTX instructions. @Artem-B did this because: ([source](llvm#67866 (comment))) > Under the hood byte extraction/insertion ends up as BFI/BFE instructions, so we may as well do that in PTX, too. https://godbolt.org/z/Tb3zWbj9b However, the example that @Artem-B linked was targeting sm_52. On modern architectures, ptxas uses prmt.b32. [Example](https://godbolt.org/z/Ye4W1n84o). Thus, remove uses of NVPTXISD::BFI in favor of NVPTXISD::PRMT.
…11454) When an OPEN statement with a unit number fails in a recoverable manner, the runtime needs to delete the ExternalFileUnit instance that was created in the unit map. And we do this too soon -- that instance still holds some of the I/O statement state that will be used by a later call into the runtime for EndIoStatement. Move the code that deletes the unit after a failed but recoverable OPEN into ExternalIoStatementBase::EndIoStatement, and don't do things afterwards that would need the I/O statement state that has been destroyed. Fixes llvm#111404.
ProgramTree instances are created as the value of a local variable in the Pre(const parser::ProgramUnit &) member function in name resolution. But references to these ProgramTree instances can persist in SubprogramNameDetails symbol table entries that might survive that function call's lifetime, and lead to trouble later when (e.g.) expression semantics needs to deal with a possible forward reference in a function reference in an expression being processed later in expression checking. So put those ProgramTree instances into a longer-lived linked list within the SemanticsContext. Might fix some weird crashes reported on big-endian targets (AIX & Solaris).
The semantics utility GetAllNames has declarations in two header files and a definition that really should be in the common utilities source file. Remove the redudant declaration from resolve-names-utils.h and move code from resolve-names-utils.cpp into Semantics/tools.cpp.
llvm#108870) This commit changes the libc++ frame recognizer to hide implementation details of libc++ more aggressively. The applied heuristic is rather straightforward: We consider every function name starting with `__` as an implementation detail. This works pretty neatly for `std::invoke`, `std::function`, `std::sort`, `std::map::emplace` and many others. Also, this should align quite nicely with libc++'s general coding convention of using the `__` for their implementation details, thereby keeping the future maintenance effort low. However, this heuristic by itself does not work in 100% of the cases: E.g., `std::ranges::sort` is not a function, but an object with an overloaded `operator()`, which means that there is no actual call `std::ranges::sort` in the call stack. Instead, there is a `std::ranges::__sort::operator()` call. To make sure that we don't hide this stack frame, we never hide the frame which represents the entry point from user code into libc++ code
Header guard was in sync with the filename.
ericastor
requested review from
dcaballe,
nicolasvasilache,
rengolin,
JDevlieghere,
Endilll and
a team
as code owners
AaronBallman
approved these changes
Oct 11, 2024
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
…ributes (llvm#110334) We already have support for declaration attributes; this is just a matter of extending the plugin infrastructure to cover one more case.
bricknerb
pushed a commit
to bricknerb/llvm-project
that referenced
this pull request
Oct 17, 2024
…ributes (llvm#110334) We already have support for declaration attributes; this is just a matter of extending the plugin infrastructure to cover one more case.
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.
We already have support for declaration attributes; this is just a matter of extending the plugin infrastructure to cover one more case.