[PATCH] Improved plugin/tool support by expanding an existing attribute
Marcwell Helpdesk via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 21 01:36:22 PST 2017
Friendly ping.
> On 14 feb 2017, at 12:05, Marcwell Helpdesk wrote:
>
> The intention of the patch is to extend the functionality of annotations while utilizing existing infrastructure in the AST and IR as much as possible. Annotations are indeed a general purpose solution, sufficient for many use cases, and a complement, not mutual exclusive to pluggable attributes. And don’t forget the realm of AST tools. The compiler should not perform any content checking of annotations but leave that to tool implementations if it is used for other purposes than merely tags. For a more complete support of annotations the AST and IR needs further work but this patch is a step on the way.
>
> Having pluggable attributes would be cool but it would face even greater problems than you describe. Unless pluggable attributes involves writing pluggable AST and IR modules the AST and IR must both support some form of generic containers that could handle any type of attribute with any number of parameters. In either case, it would most likely involve substantial architectural changes to many parts.
>
> I have revised the patch by adding a second, optional parameter that makes it possible to group annotations, reducing the risk for collisions and since it is optional it won’t break any existing code. A non-zero group string is prepended to the value string when forwarded to IR. The C++11 attribute has been moved to the clang namespace as requested and unit tests and documentation are updated to reflect the changes.
>
> Cheers,
> Chris
>
> Modified
> include/clang/Basic/Attr.td
> include/clang/Basic/AttrDocs.td
> lib/CodeGen/CodeGenFunction.cpp
> lib/CodeGen/CodeGenModule.cpp
> lib/CodeGen/CodeGenModule.h
> lib/Sema/SemaDeclAttr.cpp
> lib/Sema/SemaStmtAttr.cpp
> test/Parser/access-spec-attrs.cpp
> test/Sema/annotate.c
>
> Added
> test/CodeGenCXX/annotations-field.cpp
> test/CodeGenCXX/annotations-global.cpp
> test/CodeGenCXX/annotations-loc.cpp
> test/CodeGenCXX/annotations-var.cpp
> test/SemaCXX/attr-annotate.cpp
>
> <attr-annotate-rev2.patch>
>
>> On 9 feb 2017, at 15:07, Aaron Ballman <aaron at aaronballman.com> wrote:
>>
>> On Sat, Feb 4, 2017 at 8:26 AM, Marcwell Helpdesk via cfe-commits
>> <cfe-commits at lists.llvm.org> wrote:
>>> Many plugins/tools could benefit from having a generic way for communicating control directives directly from the source code to itself (via the AST) when acting, for example, as source code transformers, generators, collectors and the like. Attributes are a suitable way of doing this but most available attributes have predefined functionality and modifying the compiler for every plugin/tool is obviously not an option. There is however one undocumented but existing attribute that could be used for a generic solution if it was slightly modified and expanded - the annotate attribute.
>>>
>>> The current functionality of the annotate attribute encompass annotating declarations with arbitrary strings using the GNU spelling. The attached patch expands on this functionality by adding annotation of statements, C++11 spelling and documentation. With the patch applied most of the code could be annotated for the use by any Clang plugin or tool. For a more detailed description of the updated attribute, see patch for "AttrDocs.td".
>>
>> I definitely agree with the premise for this work -- having a generic
>> way for Clang to pass attributed information down to LLVM IR is
>> desirable. However, I'm not convinced that the annotate attribute is
>> the correct approach over something like pluggable attributes. My
>> primary concerns stem from the fact that the annotate attribute has no
>> safe guards for feature collisions (you have to pick a unique string,
>> or else you collide with someone else's string, but there's nothing
>> that forces you to do this), has no mechanisms for accepting arguments
>> in a consistent fashion, and provides no way to check the semantics of
>> the attribute. It's basically a minimum viable option for lowering
>> attributed information down to LLVM IR, which is fine for things that
>> aren't in the user's face, but isn't a good general-purpose solution.
>>
>> I do not have a problem with adding a C++11 spelling to the annotate
>> attribute for the cases we already support, and I definitely think we
>> should document the attribute. But I don't think it makes sense to add
>> it as a statement attribute (and the current patch also leaves out
>> type attributes).
>>
>> Some quick thoughts on the patch:
>>
>>> Index: include/clang/Basic/Attr.td
>>> ===================================================================
>>> --- include/clang/Basic/Attr.td (revision 293612)
>>> +++ include/clang/Basic/Attr.td (working copy)
>>> @@ -462,9 +462,10 @@
>>> }
>>>
>>> def Annotate : InheritableParamAttr {
>>> - let Spellings = [GNU<"annotate">];
>>> + let Spellings = [GNU<"annotate">,
>>> + CXX11<"", "annotate", 201612>];
>>
>> This should be in the clang namespace and should not be given a third
>> argument (it's not a standards-based attribute).
>>
>>> let Args = [StringArgument<"Annotation">];
>>> - let Documentation = [Undocumented];
>>> + let Documentation = [AnnotateDocs];
>>> }
>>>
>>> def ARMInterrupt : InheritableAttr, TargetSpecificAttr<TargetARM> {
>>> Index: lib/Sema/SemaStmtAttr.cpp
>>> ===================================================================
>>> --- lib/Sema/SemaStmtAttr.cpp (revision 293612)
>>> +++ lib/Sema/SemaStmtAttr.cpp (working copy)
>>> @@ -23,6 +23,24 @@
>>> using namespace clang;
>>> using namespace sema;
>>>
>>> +static Attr *handleAnnotateAttr(Sema &S, Stmt *St, const AttributeList &A,
>>> + SourceRange Range) {
>>> + // Assert string literal as the annotation's argument.
>>> + StringRef Str;
>>> + if (!S.checkStringLiteralArgumentAttr(A, 0, Str))
>>> + return nullptr;
>>> +
>>> + // Assert single argument
>>> + if (A.getNumArgs() > 1) {
>>> + S.Diag(A.getLoc(), diag::err_attribute_wrong_number_arguments)
>>> + << A.getName() << 1;
>>> + return nullptr;
>>> + }
>>> +
>>> + return ::new (S.Context) AnnotateAttr(A.getRange(), S.Context, Str,
>>> + A.getAttributeSpellingListIndex());
>>> +}
>>> +
>>> static Attr *handleFallThroughAttr(Sema &S, Stmt *St, const AttributeList &A,
>>> SourceRange Range) {
>>> FallThroughAttr Attr(A.getRange(), S.Context,
>>> @@ -273,6 +291,8 @@
>>> diag::warn_unhandled_ms_attribute_ignored :
>>> diag::warn_unknown_attribute_ignored) << A.getName();
>>> return nullptr;
>>> + case AttributeList::AT_Annotate:
>>> + return handleAnnotateAttr(S, St, A, Range);
>>> case AttributeList::AT_FallThrough:
>>> return handleFallThroughAttr(S, St, A, Range);
>>> case AttributeList::AT_LoopHint:
>>> Index: test/SemaCXX/attr-annotate.cpp
>>> ===================================================================
>>> --- test/SemaCXX/attr-annotate.cpp (nonexistent)
>>> +++ test/SemaCXX/attr-annotate.cpp (working copy)
>>> @@ -0,0 +1,17 @@
>>> +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
>>> +
>>> +[[annotate("foo")]] void foo() {
>>> + // C++11 decl annotations
>>> + [[annotate("bar")]] int x;
>>> + [[annotate(1)]] int y; // expected-error {{'annotate' attribute requires a string}}
>>> + [[annotate("bar", 1)]] int z1; // expected-error {{'annotate' attribute takes one argument}}
>>> + [[annotate()]] int z2; // expected-error {{'annotate' attribute takes one argument}}
>>> + [[annotate]] int z3; // expected-error {{'annotate' attribute takes one argument}}
>>> +
>>> + // C++11 NullStmt annotations
>>> + [[annotate("bar")]];
>>> + [[annotate(1)]]; // expected-error {{'annotate' attribute requires a string}}
>>> + [[annotate("bar", 1)]]; // expected-error {{'annotate' attribute takes one argument}}
>>> + [[annotate()]]; // expected-error {{'annotate' attribute takes one argument}}
>>> + [[annotate]]; // expected-error {{'annotate' attribute takes one argument}}
>>> +}
>>
>> There should be some codegen tests that show the attribute is properly
>> finding its way down to the LLVM IR. For instance, what would an
>> annotate attribute on a null statement get lowered to? How about a
>> switch statement?
>>
>>> Index: include/clang/Basic/AttrDocs.td
>>> ===================================================================
>>> --- include/clang/Basic/AttrDocs.td (revision 293612)
>>> +++ include/clang/Basic/AttrDocs.td (working copy)
>>> @@ -2812,3 +2812,97 @@
>>> ensure that this class cannot be subclassed.
>>> }];
>>> }
>>> +
>>> +def DocCatGeneric : DocumentationCategory<"Generic Attributes"> {
>>> + let Content = [{
>>> +These attributes are suitable for passing arbitrary information from the code to
>>> +a plugin or other clang based tools such as source code transformers, generators,
>>> +collectors and similar. The attributes are not tied to any type of solution or
>>> +predefined functionality and may be used in a variety of situations when a generic
>>> +attribute suffice.
>>> + }];
>>> +}
>>> +
>>> +def AnnotateDocs : Documentation {
>>> + let Category = DocCatGeneric;
>>> + let Content = [{
>>> +The ``annotate`` attribute may be used to annotate any type of declarations or
>>> +statements with arbitrary information that are propagated to the AST. The attribute
>>> +takes one string parameter containing the annotation value. The content, format
>>> +and the exact meaning of the value is up to the programmer to decide.
>>> +
>>> +C++11 spelling examples:
>>> +
>>> +.. code-block:: c++
>>> +
>>> + class [[annotate("plain")]] Object
>>> + { . . . };
>>> +
>>> + int main(int argc, char* argv[])
>>> + {
>>> + [[annotate("local_var")]] // Decl annotation
>>> + int n = 1 ;
>>> +
>>> + // Multiple Stmt annotations
>>> + [[annotate("group-A"), annotate("opt-1:2")]]
>>> + switch(n)
>>> + { . . . }
>>> + return 0;
>>> + }
>>> +
>>> +Looking at the AST for the example above reveals that any annotated statements
>>> +are wrapped into an AttributedStmt node together with the annotation attributes
>>> +in comparison to annotated declarations where the annotation attributes are
>>> +attached directly beneath the declaration node.
>>> +
>>> +.. code-block:: text
>>> +
>>> + TranslationUnitDecl 0x345fb20 <<invalid sloc>> <invalid sloc>
>>> + | . . .
>>> + |-CXXRecordDecl 0x34b9b58 <example.cpp:2:1, line:10:1> line:2:29 class Object definition
>>> + | |-AnnotateAttr 0x34b9c10 <col:9, col:25> "plain"
>>> + | |-CXXRecordDecl 0x34b9cb8 <col:1, col:29> col:29 implicit class Object
>>> + | |-AccessSpecDecl 0x34b9d50 <line:4:3, col:9> col:3 public
>>> + | ` . . .
>>> + `-FunctionDecl 0x34ba240 <line:12:1, line:28:1> line:12:5 main 'int (int, char **)'
>>> + |-ParmVarDecl 0x34ba020 <col:10, col:14> col:14 argc 'int'
>>> + |-ParmVarDecl 0x34ba130 <col:20, col:31> col:26 argv 'char **':'char **'
>>> + `-CompoundStmt 0x34ba7f0 <line:13:1, line:28:1>
>>> + |-DeclStmt 0x34ba4d0 <line:15:2, col:12>
>>> + | `-VarDecl 0x34ba400 <col:2, col:10> col:6 used n 'int' cinit
>>> + | |-IntegerLiteral 0x34ba4b0 <col:10> 'int' 1
>>> + | `-AnnotateAttr 0x34ba460 <line:14:4, col:24> "local_var"
>>> + |-AttributedStmt 0x34ba790 <line:17:2, line:26:2>
>>> + | |-AnnotateAttr 0x34ba750 <line:17:25, col:43> "opt-1:2"
>>> + | |-AnnotateAttr 0x34ba770 <col:4, col:22> "group-A"
>>> + | `-SwitchStmt 0x34ba608 <line:18:2, line:26:2>
>>> + | . . .
>>> + `-ReturnStmt 0x34ba7d8 <line:27:2, col:9>
>>> + `-IntegerLiteral 0x34ba7b8 <col:9> 'int' 0
>>> +
>>> +The GNU spelling, ``__attribute((annotate("str")))``, is limited to annotating
>>> +declarations but is at the same time a bit more relaxed on the allowed insertion
>>> +points compared to the C++11 spelling that needs to be defined prior to any
>>> +declaration or statement.
>>> +
>>> +In the example below the GNU-only insertion points are shown as __attr.
>>> +
>>> +.. code-block:: c++
>>> +
>>> + void __attr function/method(int __attr n) __attr
>>> + int __attr n = 1;
>>> +
>>> +(Note that it is also possible to annotate arbitrary expressions using the GCC
>>> +syntax ``int __builtin_annotation(int, "str")`` where the ``int`` is an integer
>>> +of any compiler supported bit-width and with the return value set to the same
>>> +as the parameter value. However, the annotation does not show up in the AST as
>>> +an AnnotateAttr but as an ordinary CallExpr).
>>> +
>>> +The annotations of variable, function and method declarations (and expressions)
>>> +are forwarded into IR during code generation using the LLVM annotation intrinsic
>>> +functions or by adding the information to a global annotation table. The
>>> +information added, in either case, are the annotation string, file name and
>>> +line number of the declaration or expression. These IR annotations may be used
>>> +by optimizers or other back-end tools.
>>> + }];
>>> +}
>>>
>>
>> ~Aaron
>>
>>>
>>> An example demonstratiing the use and syntax of the updated attribute:
>>>
>>> class [[annotate("plain")]] Object
>>> { . . . };
>>>
>>> int main(int argc, char* argv[]) __attribute((annotate("entry-point")))
>>> {
>>> [[annotate("local_var")]] // Decl annotation
>>> int n = 1 ;
>>>
>>> // Multiple Stmt annotations
>>> [[annotate("group-A"), annotate("opt-1:2")]]
>>> switch(n)
>>> { . . . }
>>> return 0;
>>> }
>>>
>>> Cheers,
>>> Chris
>>>
>>> Modified:
>>> include/clang/Basic/Attr.td
>>> include/clang/Basic/AttrDocs.td
>>> lib/Sema/SemaStmtAttr.cpp
>>> Added:
>>> test/SemaCXX/attr-annotate.cpp
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list