[PATCH] D144405: [clang][pp] Handle attributes defined by plugin in __has_attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Basic/Attributes.cpp:46
+  for (auto &AttrPlugin : getAttributePluginInstances()) {
+    for (auto &S : AttrPlugin->Spellings)
+      if (S.Syntax == Syntax && S.NormalizedFullName == Name)
----------------
wanders wrote:
> erichkeane wrote:
> > 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. 
> 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?
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).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144405/new/

https://reviews.llvm.org/D144405