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
