[PATCH] Downgrade 'optnone' attribute conflicts to warning

[Robinson, Paul]

r225813, thanks!

> -----Original Message-----
> From: Aaron Ballman [mailto:aaron.ballman at gmail.com]
> Sent: Tuesday, January 13, 2015 7:26 AM
> To: Robinson, Paul
> Cc: reviews+D6933+public+e7e8eb108c4fe81a at reviews.llvm.org; llvm cfe
> Subject: Re: [PATCH] Downgrade 'optnone' attribute conflicts to warning
> 
> On Tue, Jan 13, 2015 at 10:21 AM, Robinson, Paul
> <Paul_Robinson at playstation.sony.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-
> >> bounces at cs.uiuc.edu] On Behalf Of Aaron Ballman
> >> Sent: Tuesday, January 13, 2015 6:55 AM
> >> To: reviews+D6933+public+e7e8eb108c4fe81a at reviews.llvm.org
> >> Cc: llvm cfe
> >> Subject: Re: [PATCH] Downgrade 'optnone' attribute conflicts to warning
> >>
> >> On Mon, Jan 12, 2015 at 3:31 PM, Paul Robinson
> >> <Paul_Robinson at playstation.sony.com> 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