Add an attribute plugin example

[john.brawn]

Resurrecting this old patch. This is the fourth of four patches to make it possible for clang plugins to define attributes.

[aaron.ballman]

clang/examples/Attribute/Attribute.cpp

25 ↗

(On Diff #239307)

It would be really handy for the example to show how to list subjects for the declaration attribute, as that's a very common need.

29 ↗

(On Diff #239307)

Nothing here suggests the argument must be a string. It would be ideal to show users how to list out arguments and their types.

32–33 ↗

(On Diff #239307)

Showing how to have a vendor namespace would also be especially helpful.

As a point of design, I wonder if we should be documenting (or diagnosing?) attributes from plugins that are added to the clang vendor namespace, as that does not seem like something we want to encourage?

35 ↗

(On Diff #239307)

It is unclear to me how a user would add an attribute that needs to follow the merge pattern we use in mergeDeclAttribute().

53 ↗

(On Diff #239307)

Do we have a way for plugin authors to introduce their own diagnostics, or are they limited to just what diagnostics we already provide?

[sorawee]

clang/test/Frontend/plugin-attribute.cpp

16 ↗

(On Diff #239307)

Typo: scope

[john.brawn]

Update based on review comments.

[john.brawn]

clang/examples/Attribute/Attribute.cpp

25 ↗

(On Diff #239307)

This is done via diagAppertainsToDecl.

35 ↗

(On Diff #239307)

I'm guessing it's probably not possible, from a quick look at mergeDeclAttribute.

53 ↗

(On Diff #239307)

Custom diagnostics can be done with getCustomDiagID, and there's an example on line 54.

[aaron.ballman]

clang/examples/Attribute/Attribute.cpp

35 ↗

(On Diff #246989)

This is not a valid spelling for an attribute (we should probably assert that an attribute-scoped-token does not have an empty scope).

42 ↗

(On Diff #246989)

I don't think D can ever be null, can it?

35 ↗

(On Diff #239307)

That... seems like a problem that we should fix (or at least have a plan for fixing). Merging attributes is important so that we can have attributes with consistency checks between declarations for anything other than basic attribute inheritance. FWIW, there are 20+ attributes that already need this functionality, so it does get used relatively often.

[john.brawn]

Rebased and added link in documentation.

[john.brawn]

clang/examples/Attribute/Attribute.cpp

35 ↗

(On Diff #246989)

"::example" is actually how an unscoped spelling is specified - normalizeName in Basic/Attributes.cpp adds a "::" to the start if there's no scope, and the tablegen-generated getAttrKind expects it as well.

42 ↗

(On Diff #246989)

I don't know, but this is how the auto-generated diagAppertainsToDecl functions do it.

[john.brawn]

I've been looking into attribute merging, and as far as I can tell there's two things going on:

• In SemaDeclAttr.cpp, the various handleXyzAttr functions may call mergeXyzAttr to handle clashes with other attributes within the same declaration.
• In SemaDecl.cpp, several functions call mergeDeclAttributes, which calls mergeXyzAttr, to handle clashes with attributes in a previous declaration of the same thing.

For the first kind, the handleDeclAttribute function defined for a plugin attribute can check for and handle clashes with other attributes. For the second kind, at that point we're using subclasses of Attr which means we're beyond the scope of plugin attributes which just work in the realm of ParsedAttrs.

So I think things are OK, though I guess it depends on what exactly your plugin is doing - our use-case is that the plugin-defined attribute is just used to set an AnnotateAttr, which is used to tell a plugin-defined LLVM pass that it needs to do something in that function, so we don't need anything special to happen with regards to attribute merging.

[aaron.ballman]

In D31343#1902903, @john.brawn wrote:

I've been looking into attribute merging, and as far as I can tell there's two things going on:

• In SemaDeclAttr.cpp, the various handleXyzAttr functions may call mergeXyzAttr to handle clashes with other attributes within the same declaration.
• In SemaDecl.cpp, several functions call mergeDeclAttributes, which calls mergeXyzAttr, to handle clashes with attributes in a previous declaration of the same thing.

Agreed.

For the first kind, the handleDeclAttribute function defined for a plugin attribute can check for and handle clashes with other attributes.

Agreed.

For the second kind, at that point we're using subclasses of Attr which means we're beyond the scope of plugin attributes which just work in the realm of ParsedAttrs.

Yeah, we are using semantic attributes at that point rather than parsed attributes.

So I think things are OK, though I guess it depends on what exactly your plugin is doing - our use-case is that the plugin-defined attribute is just used to set an AnnotateAttr, which is used to tell a plugin-defined LLVM pass that it needs to do something in that function, so we don't need anything special to happen with regards to attribute merging.

I think things are fine for your example plugin. The scenario I'm worried about is when users want to add their own semantic attributes that need to be merged, or if they're using an existing semantic attribute but need to add custom merging logic to it for their own needs.

[john.brawn]

Updated to match the changes to D31342

[aaron.ballman]

clang/examples/Attribute/Attribute.cpp

35 ↗

(On Diff #246989)

It's not how an unscoped spelling is specified by either C or C++ -- in both languages, that would be an error. I don't think we want to leak this implementation detail out to the user as it will be confusing to them.

42 ↗

(On Diff #246989)

I'll fix that up -- I just verified that D can never be null. Sema::ProcessDeclAttributeList dereferences D unconditionally and has for a long time.

[Jasmin]

Hello John!

I was encouraged by Erich after a talk I had with him and Aaron on IRC to contact you about an use case for this plugin.

I wanted to use user-specified/unknown attributes to annotate code and was told that they are not propagated through
the AST and therefore it is not possible at the moment to use them for that purpose. Aaron's and Erich's idea was to
allow kind of no-op attributes in the plugin so that the compiler would not issue a warning about the unknown attribute.
This would be helpful for the user being able to define a list of attributes the user knows and expects, so that a compilation
would not be spammed unnecessarily and misspelled attributes would still stand out.

Being able to get the cursor or displayName of the attribute would essentially be enough to use it in tooling.

We can use attribute((annotate(smth))) at the time being to achieve the same goal, but it is much more writing to do
and also vendor specific. Being able to do the same with attributes would give them a real purpose, other than having
to be accepted and not causing an error. Also they have to be supported by the language and we don't have to use
macros to annotate the code.

To summarize it would be nice to have:

• user supplied unknown attributes to suppress a warning
• unknown attributes propagated in the AST

I hope I summarized this correctly and could get through the gist of this idea.

Looking forward to hearing from you.

Best regards, Jasmin

[aaron.ballman]

In D31343#1936810, @Jasmin wrote:

Hello John!

I was encouraged by Erich after a talk I had with him and Aaron on IRC to contact you about an use case for this plugin.

I wanted to use user-specified/unknown attributes to annotate code and was told that they are not propagated through
the AST and therefore it is not possible at the moment to use them for that purpose. Aaron's and Erich's idea was to
allow kind of no-op attributes in the plugin so that the compiler would not issue a warning about the unknown attribute.
This would be helpful for the user being able to define a list of attributes the user knows and expects, so that a compilation
would not be spammed unnecessarily and misspelled attributes would still stand out.

Being able to get the cursor or displayName of the attribute would essentially be enough to use it in tooling.

We can use attribute((annotate(smth))) at the time being to achieve the same goal, but it is much more writing to do
and also vendor specific. Being able to do the same with attributes would give them a real purpose, other than having
to be accepted and not causing an error. Also they have to be supported by the language and we don't have to use
macros to annotate the code.

To summarize it would be nice to have:

• user supplied unknown attributes to suppress a warning
• unknown attributes propagated in the AST

I hope I summarized this correctly and could get through the gist of this idea.

Looking forward to hearing from you.

For reference: the current set of patches John has been working on allow you to load a plugin to let the frontend parse an attribute with an arbitrary spelling, but it does not have a way for you to register a new attribute AST node that gets attached to anything in the AST. Instead, you create an existing semantic attribute (such as an AnnotateAttr) and add that to the AST.

I think this is a great request to keep in mind for John or anyone else who wants to extend this plugin functionality to allow for generating custom attribute AST nodes (which is of interest for tools like clang static analyzer or clang tidy plugins that may consume the clang AST). This functionality would also have to keep the libclang C indexing interface in mind, as that's another area of the compiler where this functionality would be useful.

[erichkeane]

In D31343#1937588, @aaron.ballman wrote:

In D31343#1936810, @Jasmin wrote:

Hello John!

I was encouraged by Erich after a talk I had with him and Aaron on IRC to contact you about an use case for this plugin.

I wanted to use user-specified/unknown attributes to annotate code and was told that they are not propagated through
the AST and therefore it is not possible at the moment to use them for that purpose. Aaron's and Erich's idea was to
allow kind of no-op attributes in the plugin so that the compiler would not issue a warning about the unknown attribute.
This would be helpful for the user being able to define a list of attributes the user knows and expects, so that a compilation
would not be spammed unnecessarily and misspelled attributes would still stand out.

Being able to get the cursor or displayName of the attribute would essentially be enough to use it in tooling.

We can use attribute((annotate(smth))) at the time being to achieve the same goal, but it is much more writing to do
and also vendor specific. Being able to do the same with attributes would give them a real purpose, other than having
to be accepted and not causing an error. Also they have to be supported by the language and we don't have to use
macros to annotate the code.

To summarize it would be nice to have:

• user supplied unknown attributes to suppress a warning
• unknown attributes propagated in the AST

I hope I summarized this correctly and could get through the gist of this idea.

Looking forward to hearing from you.

For reference: the current set of patches John has been working on allow you to load a plugin to let the frontend parse an attribute with an arbitrary spelling, but it does not have a way for you to register a new attribute AST node that gets attached to anything in the AST. Instead, you create an existing semantic attribute (such as an AnnotateAttr) and add that to the AST.

I think this is a great request to keep in mind for John or anyone else who wants to extend this plugin functionality to allow for generating custom attribute AST nodes (which is of interest for tools like clang static analyzer or clang tidy plugins that may consume the clang AST). This functionality would also have to keep the libclang C indexing interface in mind, as that's another area of the compiler where this functionality would be useful.

Hmm... I wonder if there is value to having these plugin-attributes auto-create a PluginAnnotateAttr or something (some otherwise bikeshedded attribute) for exactly Jasmin's use case. That way the plugin author simply needs to list the possible spellings.

[john.brawn]

A few small changes:

• Don't start C++11 spelling with :: (changes to name normalisation to allow this are in D76704)
• Don't check that the Decl is null in diagAppertainsToDecl
• Return AttributeNotApplied on errors in handleDeclAttribute

[aaron.ballman]

LGTM, thank you!

[aaron.ballman]

I think this was the last remaining review for the feature to support attributes as plugins, yes? If so, can you please add a release note about the feature to the release notes?

[john.brawn]

Release note added: https://reviews.llvm.org/rG0cff81cff05d

examples/Attribute/Attribute.cpp

+//===- Attribute.cpp ------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Example clang plugin which adds an an annotation to file-scope declarations
+// with the 'example' attribute.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/Sema/AttributeList.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/IR/Attributes.h"
+using namespace clang;
+
+namespace {
+
+struct ExampleAttrInfo : public ParsedAttrInfo {
+  ExampleAttrInfo() {
+    Syntax = AttributeList::AS_GNU;
+    AttrKind = AttributeList::AT_Annotate;
+    OptArgs = 1;
+  }
+  virtual bool handleDeclAttribute(Sema &S, Decl *D, const AttributeList &Attr) {
+    /* Check if the decl is at file scope */
+    if (!D->getDeclContext()->isFileContext()) {
+      unsigned ID = S.getDiagnostics().getCustomDiagID(
+        DiagnosticsEngine::Error,
+        "'example' attribute only allowed at file scope");
+      S.Diag(Attr.getLoc(), ID);
+    }
+    /* The attribute can take an optional string argument: check for this */
+    StringRef Str = "";
+    if (Attr.getNumArgs() > 0) {
+      Expr *ArgExpr = Attr.getArgAsExpr(0);
+      StringLiteral *Literal =
+          dyn_cast<StringLiteral>(ArgExpr->IgnoreParenCasts());
+      if (Literal) {
+        Str = Literal->getString();
+      } else {
+        S.Diag(ArgExpr->getLocStart(), diag::err_attribute_argument_type)
+          << Attr.getName() << AANT_ArgumentString;
+      }
+    }
+    /* Attach an annotate attribute to the Decl */
+    D->addAttr(new (S.Context)AnnotateAttr(Attr.getRange(), S.Context,
+                                           "example("+Str.str()+")", 0));
+    return true;
+  }
+};
+
+}
+
+static AttrInfoRegistry::Add<ExampleAttrInfo> X("example", "");

examples/Attribute/CMakeLists.txt

+add_llvm_loadable_module(Attribute Attribute.cpp PLUGIN_TOOL clang)
+
+if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
+  target_link_libraries(AnnotateFunctions ${cmake_2_8_12_PRIVATE}
+    clangAST
+    clangBasic
+    clangFrontend
+    clangLex
+    LLVMSupport
+    )
+endif()

examples/CMakeLists.txt

 if(NOT CLANG_BUILD_EXAMPLES)
   set_property(DIRECTORY PROPERTY EXCLUDE_FROM_ALL ON)
   set(EXCLUDE_FROM_ALL ON)
 endif()
 
 if(CLANG_ENABLE_STATIC_ANALYZER)
 add_subdirectory(analyzer-plugin)
 endif()
 add_subdirectory(clang-interpreter)
 add_subdirectory(PrintFunctionNames)
 add_subdirectory(AnnotateFunctions)
+add_subdirectory(Attribute)

test/CMakeLists.txt

if (CLANG_ENABLE_ARCMT)
   list(APPEND CLANG_TEST_DEPS
     arcmt-test
     c-arcmt-test
   )
 endif ()
 
 if (CLANG_BUILD_EXAMPLES)
   list(APPEND CLANG_TEST_DEPS
+    Attribute
     AnnotateFunctions
     clang-interpreter
     PrintFunctionNames
     )
 endif ()
 
 if (CLANG_ENABLE_STATIC_ANALYZER AND CLANG_BUILD_EXAMPLES)
   list(APPEND CLANG_TEST_DEPS

test/Frontend/plugin-attribute.c

+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm -S %s -o - | FileCheck %s --check-prefix=ATTRIBUTE
+// RUN: not %clang -fplugin=%llvmshlibdir/Attribute%pluginext -emit-llvm -DBAD_ATTRIBUTE -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADATTRIBUTE
+// REQUIRES: plugins, examples
+
+// ATTRIBUTE: [[STR1_VAR:@.+]] = private unnamed_addr constant [10 x i8] c"example()\00"
+// ATTRIBUTE: [[STR2_VAR:@.+]] = private unnamed_addr constant [20 x i8] c"example(somestring)\00"
+// ATTRIBUTE: [[STR3_VAR:@.+]] = private unnamed_addr constant [21 x i8] c"example(otherstring)\00"
+// ATTRIBUTE: @llvm.global.annotations = {{.*}}@fn1{{.*}}[[STR1_VAR]]{{.*}}@fn2{{.*}}[[STR2_VAR]]{{.*}}@var1{{.*}}[[STR3_VAR]]
+void fn1() __attribute__((example)) { }
+void fn2() __attribute__((example("somestring"))) { }
+int var1 __attribute__((example("otherstring"))) = 1;
+
+#ifdef BAD_ATTRIBUTE
+void fn3() {
+  // BADATTRIBUTE: error: 'example' attribute only allowed at file scop
+  int var2 __attribute__((example));
+}
+// BADATTRIBUTE: error: 'example' attribute requires a string
+void fn4() __attribute__((example(123))) { }
+// BADATTRIBUTE: error: 'example' attribute takes no more than 1 argument
+void fn5() __attribute__((example("a","b"))) { }
+#endif