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 <aa...@aaronballman.com> wrote: >> >> On Sat, Feb 4, 2017 at 8:26 AM, Marcwell Helpdesk via cfe-commits >> <cfe-commits@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@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits