[clang][pp] Handle attributes defined by plugin in __has_attribute

[wanders]

Summary

When using attributes by plugins (both in clang and clang-tidy) the
preprocessor functions __has_attribute, __has_c_attribute,
__has_cpp_attribute still returned 0.

That problem is fixed by having the "hasAttribute" function also check
if any of the plugins provide that attribute.

This also adds C2x spelling to the example plugin for attributes so that
__has_c_attribute can be tested.

[erichkeane]

This needs a release note. Otherwise I just have a preference for the 'is this spelling/syntax combo provided by this attribute' being in the plugin infrastructure. It DOES make me wonder however, what happens if TWO plugins provide a different 'true' answer for those?

clang/lib/Basic/Attributes.cpp

40

Ah, I see what you've done here. I guess that is OK on the other patch then.

46

I think this should be a property of the AttributePlugin type, where you send it the Syntax/Name and it checks to see if it is provided.

[wanders]

clang/lib/Basic/Attributes.cpp

46

Do you think I should do something like this?

(refactor based on mainline)

diff --git a/clang/include/clang/Sema/ParsedAttr.h b/clang/include/clang/Sema/ParsedAttr.h
index f060564e6719..635580ac599b 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -98,6 +98,13 @@ protected:
 public:
   virtual ~ParsedAttrInfo() = default;
 
+  /// Check if this attribute can be spelled like this. Only used for plugin attributes
+  virtual bool hasSpelling(AttributeCommonInfo::Syntax Syntax, StringRef Name) {
+    return llvm::any_of(Spellings, [&] (const Spelling &S) {
+      return (S.Syntax == Syntax && S.NormalizedFullName == Name);
+    });
+  }
+
   /// Check if this attribute appertains to D, and issue a diagnostic if not.
   virtual bool diagAppertainsToDecl(Sema &S, const ParsedAttr &Attr,
                                     const Decl *D) const {
diff --git a/clang/lib/Sema/ParsedAttr.cpp b/clang/lib/Sema/ParsedAttr.cpp
index c1e39acb14ec..b53132d73d79 100644
--- a/clang/lib/Sema/ParsedAttr.cpp
+++ b/clang/lib/Sema/ParsedAttr.cpp
@@ -135,8 +135,7 @@ const ParsedAttrInfo &ParsedAttrInfo::get(const AttributeCommonInfo &A) {
     SyntaxUsed = AttributeCommonInfo::AS_Keyword;
 
   for (auto &Ptr : *PluginAttrInstances)
-    for (auto &S : Ptr->Spellings)
-      if (S.Syntax == SyntaxUsed && S.NormalizedFullName == FullName)
+    if (Ptr->handlesSpelling(SyntaxUsed, FullName))
         return *Ptr;
 
   // If we failed to find a match then return a default ParsedAttrInfo.

Maybe not virtual as allowing plugins to handle attributes not in their Spellings array maybe is too flexible?

[erichkeane]

clang/lib/Basic/Attributes.cpp

46

Hi-
Sorry for the delayed response: you caught me out of the office.

Yes, that is what I meant. I would NOT make it virtual (for the reason you stated).

[wanders]

@erichkeane
Patch stack now updated to include a method extraction of hasSpelling, and releasenote added.

[erichkeane]

Looks like this doesn't compile pre-commit, though no idea if that is a patch-stack issue. Other than test, patch looks fine.

clang/test/Frontend/plugin-attribute-pp.cpp

2

This split-file thing is weird, don't do that. create 2 tests instead, OR Just use -x c and -x c++ to change languages (you can use a separate macro to get has_c and has_cpp_attribute right).

[wanders]

In D144405#4184726, @erichkeane wrote:

Looks like this doesn't compile pre-commit, though no idea if that is a patch-stack issue. Other than test, patch looks fine.

Yeah, when uploading the new commit the stack was disorganized. So the build probably started before I got around organizing the stack in correct order.

So hopefully clears up when I fix the test (clang-format and const thing)

clang/test/Frontend/plugin-attribute-pp.cpp

2

Right. That should be simpler. Will fix.

(it ended up this way because I first added a split to the existing plugin-attribute.cpp test with splits)

[erichkeane]

So here's a potential idea for future development: It isn't uncommon/untypical for an attribute to want to return something other than '1', for 'version' (usually an integral value representing a date). The standard attributes all do this. It might be worth looking into some infrastructure to do that.

[wanders]

In D144405#4184964, @erichkeane wrote:

So here's a potential idea for future development: It isn't uncommon/untypical for an attribute to want to return something other than '1', for 'version' (usually an integral value representing a date). The standard attributes all do this. It might be worth looking into some infrastructure to do that.

Adding this to my list of things to look into. Thanks.

I see that D144403 in the stack is not accepted, is there anything I need to fix that I have missed?

My current plan after this is to look into making it possible to attach custom attributes to the AST nodes, and in the end use plugin-defined attributes in ASTMatchers.

So I'll probably be poking around in that area anyway.

My imaginary API for that is something like:

    /* find all calls to functions with my-attribute from functions without my-attribute
    Finder->addMatcher(callExpr(
				callee(
				       functionDecl(
						    hasAttr(MyAttribute)
						    )
				       )
				hasAncestor(
					    functionDecl(
							 unless(hasAttr(MyAttribute))
							 ).bind("containing-function")
					    )
				).bind("call"), this);

where "MyAttribute" is the plugin defined attribute.

clang/docs/ReleaseNotes.rst

 - ``__declspec`` attributes can now be used together with the using keyword. Before
   the attributes on ``__declspec`` was ignored, while now it will be forwarded to the
   point where the alias is used.
 - Introduced a new ``USR`` (unified symbol resolution) clause inside of the
   existing ``__attribute__((external_source_symbol))`` attribute. Clang's indexer
   uses the optional USR value when indexing Clang's AST. This value is expected
   to be generated by an external compiler when generating C++ bindings during
   the compilation of the foreign language sources (e.g. Swift).
+- The ``__has_attribute``, ``__has_c_attribute`` and ``__has_cpp_attribute``
+  preprocessor operators now return 1 also for attributes defined by plugins.
 
 Improvements to Clang's diagnostics
 -----------------------------------
 - We now generate a diagnostic for signed integer overflow due to unary minus
   in a non-constant expression context.
   (`#31643 <https://github.com/llvm/llvm-project/issues/31643>`_)
 - Clang now warns by default for C++20 and later about deprecated capture of
   ``this`` with a capture default of ``=``. This warning can be disabled with

clang/examples/Attribute/Attribute.cpp

 //===- Attribute.cpp ------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Example clang plugin which adds an an annotation to file-scope declarations
 // with the 'example' attribute.
 //
+// This plugin is used by clang/test/Frontend/plugin-attribute tests.
+//
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/Sema/ParsedAttr.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() {
     // Can take up to 15 optional arguments, to emulate accepting a variadic
     // number of arguments. This just illustrates how many arguments a
     // `ParsedAttrInfo` can hold, we will not use that much in this example.
     OptArgs = 15;
-    // GNU-style __attribute__(("example")) and C++-style [[example]] and
+    // GNU-style __attribute__(("example")) and C++/C2x-style [[example]] and
     // [[plugin::example]] supported.
     static constexpr Spelling S[] = {{ParsedAttr::AS_GNU, "example"},
+                                     {ParsedAttr::AS_C2x, "example"},
                                      {ParsedAttr::AS_CXX11, "example"},
                                      {ParsedAttr::AS_CXX11, "plugin::example"}};
     Spellings = S;
   }
 
   bool diagAppertainsToDecl(Sema &S, const ParsedAttr &Attr,
                             const Decl *D) const override {
     // This attribute appertains to functions only.

clang/lib/Basic/Attributes.cpp

 #include "clang/Basic/Attributes.h"
 #include "clang/Basic/AttrSubjectMatchRules.h"
 #include "clang/Basic/AttributeCommonInfo.h"
 #include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/ParsedAttrInfo.h"
 using namespace clang;
 
 static int hasAttributeImpl(AttributeCommonInfo::Syntax Syntax, StringRef Name,
                             StringRef ScopeName, const TargetInfo &Target,
                             const LangOptions &LangOpts) {
 
 #include "clang/Basic/AttrHasAttributeImpl.inc"
 
int clang::hasAttribute(AttributeCommonInfo::Syntax Syntax,
   // As a special case, look for the omp::sequence and omp::directive
   // attributes. We support those, but not through the typical attribute
   // machinery that goes through TableGen. We support this in all OpenMP modes
   // so long as double square brackets are enabled.
   if (LangOpts.OpenMP && LangOpts.DoubleSquareBracketAttributes &&
       ScopeName == "omp")
     return (Name == "directive" || Name == "sequence") ? 1 : 0;
 
   int res = hasAttributeImpl(Syntax, Name, ScopeName, Target, LangOpts);
   if (res)
     return res;
 
+  // Check if any plugin provides this attribute.
+  for (auto &Ptr : getAttributePluginInstances())
+    if (Ptr->hasSpelling(Syntax, Name))
+      return 1;
+
   return 0;
 }
 
 const char *attr::getSubjectMatchRuleSpelling(attr::SubjectMatchRule Rule) {
   switch (Rule) {
 #define ATTR_MATCH_RULE(NAME, SPELLING, IsAbstract)                            \
   case attr::NAME:                                                             \
     return SPELLING;

clang/test/Frontend/plugin-attribute-pp.cpp

+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -E %s | FileCheck %s
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -E %s -x c | FileCheck %s
+// REQUIRES: plugins, examples
+
+#ifdef __cplusplus
+# define HAS_ATTR(a) __has_cpp_attribute (a)
+#else
+# define HAS_ATTR(a) __has_c_attribute (a)
+#endif
+
+#if __has_attribute(example)
+// CHECK: has_attribute(example) was true
+has_attribute(example) was true
+#endif
+#if HAS_ATTR(example)
+// CHECK: has_$LANG_attribute(example) was true
+has_$LANG_attribute(example) was true
+#endif
+
+#if __has_attribute(doesnt_exist)
+// CHECK-NOT: has_attribute(doesnt_exist) unexpectedly was true
+has_attribute(doesnt_exist) unexpectedly was true
+#endif
+
+#if HAS_ATTR(doesnt_exist)
+// CHECK-NOT: has_$LANG_attribute(doesnt_exist) unexpectedly was true
+has_$LANG_attribute(doesnt_exist) unexpectedly was true
+#endif