r225813, thanks! > -----Original Message----- > From: Aaron Ballman [mailto:[email protected]] > Sent: Tuesday, January 13, 2015 7:26 AM > To: Robinson, Paul > Cc: [email protected]; llvm cfe > Subject: Re: [PATCH] Downgrade 'optnone' attribute conflicts to warning > > On Tue, Jan 13, 2015 at 10:21 AM, Robinson, Paul > <[email protected]> wrote: > > > > > >> -----Original Message----- > >> From: [email protected] [mailto:cfe-commits- > >> [email protected]] On Behalf Of Aaron Ballman > >> Sent: Tuesday, January 13, 2015 6:55 AM > >> To: [email protected] > >> Cc: llvm cfe > >> Subject: Re: [PATCH] Downgrade 'optnone' attribute conflicts to warning > >> > >> On Mon, Jan 12, 2015 at 3:31 PM, Paul Robinson > >> <[email protected]> wrote: > >> > Hi aaron.ballman, > >> > > >> > When attribute 'optnone' appears on the same declaration with a > >> > conflicting attribute, warn about the conflict and pick a "winning" > >> > attribute to preserve, instead of emitting an error. This matches > the > >> > behavior when the conflicting attributes are on different > declarations. > >> > > >> > Along the way I discovered that conflicts involving __forceinline > were > >> > reported as 'always_inline' (alternate spellings for the same > >> > attribute) so I hacked in a way to provide the spelling used in the > >> > declaration. There must be a better way, though... > >> > >> LGTM, with one possible suggestion below. > >> > >> > > >> > http://reviews.llvm.org/D6933 > >> > > >> > Files: > >> > include/clang/Sema/Sema.h > >> > lib/Sema/SemaDecl.cpp > >> > lib/Sema/SemaDeclAttr.cpp > >> > test/SemaCXX/attr-optnone.cpp > >> > > >> > EMAIL PREFERENCES > >> > http://reviews.llvm.org/settings/panel/emailpreferences/ > >> > >> > Index: include/clang/Sema/Sema.h > >> > =================================================================== > >> > --- include/clang/Sema/Sema.h > >> > +++ include/clang/Sema/Sema.h > >> > @@ -2009,6 +2009,7 @@ > >> > SectionAttr *mergeSectionAttr(Decl *D, SourceRange Range, > StringRef > >> Name, > >> > unsigned AttrSpellingListIndex); > >> > AlwaysInlineAttr *mergeAlwaysInlineAttr(Decl *D, SourceRange > Range, > >> > + StringRef Name, > >> > unsigned > >> AttrSpellingListIndex); > >> > MinSizeAttr *mergeMinSizeAttr(Decl *D, SourceRange Range, > >> > unsigned AttrSpellingListIndex); > >> > Index: lib/Sema/SemaDecl.cpp > >> > =================================================================== > >> > --- lib/Sema/SemaDecl.cpp > >> > +++ lib/Sema/SemaDecl.cpp > >> > @@ -2155,7 +2155,8 @@ > >> > AttrSpellingListIndex, > >> > IA->getSemanticSpelling()); > >> > else if (const auto *AA = dyn_cast<AlwaysInlineAttr>(Attr)) > >> > - NewAttr = S.mergeAlwaysInlineAttr(D, AA->getRange(), > >> AttrSpellingListIndex); > >> > + NewAttr = S.mergeAlwaysInlineAttr(D, AA->getRange(), AA- > >> >getSpelling(), > >> > + AttrSpellingListIndex); > >> > else if (const auto *MA = dyn_cast<MinSizeAttr>(Attr)) > >> > NewAttr = S.mergeMinSizeAttr(D, MA->getRange(), > >> AttrSpellingListIndex); > >> > else if (const auto *OA = dyn_cast<OptimizeNoneAttr>(Attr)) > >> > Index: lib/Sema/SemaDeclAttr.cpp > >> > =================================================================== > >> > --- lib/Sema/SemaDeclAttr.cpp > >> > +++ lib/Sema/SemaDeclAttr.cpp > >> > @@ -3141,9 +3141,14 @@ > >> > } > >> > > >> > AlwaysInlineAttr *Sema::mergeAlwaysInlineAttr(Decl *D, SourceRange > >> Range, > >> > + StringRef Name, > >> > unsigned > >> AttrSpellingListIndex) { > >> > if (OptimizeNoneAttr *Optnone = D->getAttr<OptimizeNoneAttr>()) { > >> > - Diag(Range.getBegin(), diag::warn_attribute_ignored) << > >> "'always_inline'"; > >> > + SmallString<40> QuotedName; > >> > + QuotedName = "'"; > >> > + QuotedName += Name; > >> > + QuotedName += "'"; > >> > + Diag(Range.getBegin(), diag::warn_attribute_ignored) << > QuotedName; > >> > Diag(Optnone->getLocation(), diag::note_conflicting_attribute); > >> > return nullptr; > >> > } > >> > >> You could pass in the name as an IdentifierInfo * and ditch the manual > >> quoting. I realize that Attr::getSpelling() returns a StringRef, but > >> getting the IdentifierInfo should be low-cost and slightly less ugly. > > > > Yes, with an IdentifierInfo then I just << it into Diag and the quoting > > happens for me, definitely cleaner. > > > > That works when calling mergeAlwaysInlineAttr from > handleAlwaysInlineAttr, > > but it's also called from mergeDeclAttribute in SemaDecl.cpp. In that > case > > I have an InheritableAttr, not an AttributeList, and I didn't see a way > to > > get an IdentifierInfo out of it. That's why I went with passing the > string. > > > > Is there a way to get IdentifierInfo out of InheritableAttr? > > Context.Idents.get(theAttr->getSpelling()) > > ~Aaron
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
