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. > @@ -3191,34 +3196,23 @@ > > static void handleAlwaysInlineAttr(Sema &S, Decl *D, > const AttributeList &Attr) { > - if (checkAttrMutualExclusion<OptimizeNoneAttr>(S, D, Attr)) > - return; > - > - D->addAttr(::new (S.Context) > - AlwaysInlineAttr(Attr.getRange(), S.Context, > - Attr.getAttributeSpellingListIndex())); > + if (AlwaysInlineAttr *Inline = S.mergeAlwaysInlineAttr( > + D, Attr.getRange(), Attr.getName()->getName(), > + Attr.getAttributeSpellingListIndex())) > + D->addAttr(Inline); > } > > -static void handleMinSizeAttr(Sema &S, Decl *D, > - const AttributeList &Attr) { > - if (checkAttrMutualExclusion<OptimizeNoneAttr>(S, D, Attr)) > - return; > - > - D->addAttr(::new (S.Context) > - MinSizeAttr(Attr.getRange(), S.Context, > - Attr.getAttributeSpellingListIndex())); > +static void handleMinSizeAttr(Sema &S, Decl *D, const AttributeList &Attr) { > + if (MinSizeAttr *MinSize = S.mergeMinSizeAttr( > + D, Attr.getRange(), Attr.getAttributeSpellingListIndex())) > + D->addAttr(MinSize); > } > > static void handleOptimizeNoneAttr(Sema &S, Decl *D, > const AttributeList &Attr) { > - if (checkAttrMutualExclusion<AlwaysInlineAttr>(S, D, Attr)) > - return; > - if (checkAttrMutualExclusion<MinSizeAttr>(S, D, Attr)) > - return; > - > - D->addAttr(::new (S.Context) > - OptimizeNoneAttr(Attr.getRange(), S.Context, > - Attr.getAttributeSpellingListIndex())); > + if (OptimizeNoneAttr *Optnone = S.mergeOptimizeNoneAttr( > + D, Attr.getRange(), Attr.getAttributeSpellingListIndex())) > + D->addAttr(Optnone); > } > > static void handleGlobalAttr(Sema &S, Decl *D, const AttributeList &Attr) { > Index: test/SemaCXX/attr-optnone.cpp > =================================================================== > --- test/SemaCXX/attr-optnone.cpp > +++ test/SemaCXX/attr-optnone.cpp > @@ -3,17 +3,17 @@ > int foo() __attribute__((optnone)); > int bar() __attribute__((optnone)) __attribute__((noinline)); > > -int baz() __attribute__((always_inline)) __attribute__((optnone)); // > expected-error{{'always_inline' and 'optnone' attributes are not compatible}} > -int quz() __attribute__((optnone)) __attribute__((always_inline)); // > expected-error{{'optnone' and 'always_inline' attributes are not compatible}} > +int baz() __attribute__((always_inline)) __attribute__((optnone)); // > expected-warning{{'always_inline' attribute ignored}} > expected-note{{conflicting attribute is here}} > +int quz() __attribute__((optnone)) __attribute__((always_inline)); // > expected-warning{{'always_inline' attribute ignored}} > expected-note{{conflicting attribute is here}} > > __attribute__((always_inline)) int baz1(); // > expected-warning{{'always_inline' attribute ignored}} > __attribute__((optnone)) int baz1() { return 1; } // > expected-note{{conflicting attribute is here}} > > __attribute__((optnone)) int quz1(); // expected-note{{conflicting attribute > is here}} > __attribute__((always_inline)) int quz1() { return 1; } // > expected-warning{{'always_inline' attribute ignored}} > > -int bay() __attribute__((minsize)) __attribute__((optnone)); // > expected-error{{'minsize' and 'optnone' attributes are not compatible}} > -int quy() __attribute__((optnone)) __attribute__((minsize)); // > expected-error{{'optnone' and 'minsize' attributes are not compatible}} > +int bay() __attribute__((minsize)) __attribute__((optnone)); // > expected-warning{{'minsize' attribute ignored}} expected-note{{conflicting}} > +int quy() __attribute__((optnone)) __attribute__((minsize)); // > expected-warning{{'minsize' attribute ignored}} expected-note{{conflicting}} > > __attribute__((minsize)) int bay1(); // expected-warning{{'minsize' > attribute ignored}} > __attribute__((optnone)) int bay1() { return 1; } // > expected-note{{conflicting attribute is here}} > @@ -27,8 +27,13 @@ > __attribute__((optnone)) // expected-note 2 {{conflicting}} > void bay2() {} > > -__forceinline __attribute__((optnone)) int bax(); // > expected-error{{'__forceinline' and 'optnone' attributes are not compatible}} > -__attribute__((optnone)) __forceinline int qux(); // > expected-error{{'optnone' and '__forceinline' attributes are not compatible}} > +__forceinline __attribute__((optnone)) int bax(); // > expected-warning{{'__forceinline' attribute ignored}} > expected-note{{conflicting}} > +__attribute__((optnone)) __forceinline int qux(); // > expected-warning{{'__forceinline' attribute ignored}} > expected-note{{conflicting}} > + > +__forceinline int bax2(); // expected-warning{{'__forceinline' attribute > ignored}} > +__attribute__((optnone)) int bax2() { return 1; } // > expected-note{{conflicting}} > +__attribute__((optnone)) int qux2(); // expected-note{{conflicting}} > +__forceinline int qux2() { return 1; } // expected-warning{{'__forceinline' > attribute ignored}} > > int globalVar __attribute__((optnone)); // expected-warning{{'optnone' > attribute only applies to functions}} > > @@ -50,8 +55,8 @@ > [[clang::optnone]] > int bar2() __attribute__((noinline)); > > -[[clang::optnone]] > -int baz2() __attribute__((always_inline)); // > expected-error{{'always_inline' and 'optnone' attributes are not compatible}} > +[[clang::optnone]] // expected-note {{conflicting}} > +int baz2() __attribute__((always_inline)); // > expected-warning{{'always_inline' attribute ignored}} > > [[clang::optnone]] int globalVar2; //expected-warning{{'optnone' attribute > only applies to functions}} > > ~Aaron _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
