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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits