I just realized this might seem a little weird, since we'll still give a -Wunused-value warning for an unused comparison in a macro body. But, my next patch is going to be "suppress all -Wunused-value warnings from expressions in macro bodies" -- these are the biggest source of false positives from -Wunused-value in Google's codebase.
On Wed, Jan 2, 2013 at 5:10 PM, Matt Beaumont-Gay <[email protected]> wrote: > Hi chandlerc, > > Tweaking the implementation of -Wunused-value, I noticed a misbehavior of > -Wunused-comparison: If the comparison is in a macro argument, we give up on > emitting -Wunused-comparison, but fall through to emitting -Wunused-value. > This patch fixes -Wunused-comparison to do the right thing. I also added a > helper function 'isMacroBodyExpansion' to SourceManager (to go along with > 'isMacroArgExpansion'). > > http://llvm-reviews.chandlerc.com/D259 > > Files: > include/clang/Basic/SourceManager.h > lib/Basic/SourceManager.cpp > lib/Sema/SemaStmt.cpp > test/Sema/unused-expr-system-header.c > > Index: include/clang/Basic/SourceManager.h > =================================================================== > --- include/clang/Basic/SourceManager.h > +++ include/clang/Basic/SourceManager.h > @@ -329,6 +329,11 @@ > SourceLocation::getFromRawEncoding(ExpansionLocEnd).isInvalid(); > } > > + bool isMacroBodyExpansion() const { > + return getExpansionLocStart().isValid() && > + SourceLocation::getFromRawEncoding(ExpansionLocEnd).isValid(); > + } > + > bool isFunctionMacroExpansion() const { > return getExpansionLocStart().isValid() && > getExpansionLocStart() != getExpansionLocEnd(); > @@ -1126,6 +1131,11 @@ > /// expanded. > bool isMacroArgExpansion(SourceLocation Loc) const; > > + /// \brief Tests whether the given source location represents the > expansion of > + /// a macro body (as opposed to an expansion of the argument to a > + /// function-like macro). > + bool isMacroBodyExpansion(SourceLocation Loc) const; > + > /// \brief Returns true if \p Loc is inside the [\p Start, +\p Length) > /// chunk of the source location address space. > /// > Index: lib/Basic/SourceManager.cpp > =================================================================== > --- lib/Basic/SourceManager.cpp > +++ lib/Basic/SourceManager.cpp > @@ -979,6 +979,15 @@ > return Expansion.isMacroArgExpansion(); > } > > +bool SourceManager::isMacroBodyExpansion(SourceLocation Loc) const { > + if (!Loc.isMacroID()) return false; > + > + FileID FID = getFileID(Loc); > + const SrcMgr::SLocEntry *E = &getSLocEntry(FID); > + const SrcMgr::ExpansionInfo &Expansion = E->getExpansion(); > + return Expansion.isMacroBodyExpansion(); > +} > + > > > //===----------------------------------------------------------------------===// > // Queries about the code at a SourceLocation. > Index: lib/Sema/SemaStmt.cpp > =================================================================== > --- lib/Sema/SemaStmt.cpp > +++ lib/Sema/SemaStmt.cpp > @@ -125,7 +125,7 @@ > > // Suppress warnings when the operator, suspicious as it may be, comes from > // a macro expansion. > - if (Loc.isMacroID()) > + if (S.SourceMgr.isMacroBodyExpansion(Loc)) > return false; > > S.Diag(Loc, diag::warn_unused_comparison) > Index: test/Sema/unused-expr-system-header.c > =================================================================== > --- test/Sema/unused-expr-system-header.c > +++ test/Sema/unused-expr-system-header.c > @@ -3,8 +3,10 @@ > void f(int i1, int i2) { > POSSIBLY_BAD_MACRO(5); > STATEMENT_EXPR_MACRO(5); > - COMMA_MACRO_1(i1 == i2, f(i1, i2)); // expected-warning {{expression > result unused}} > + COMMA_MACRO_1(i1 == i2, f(i1, i2)); // expected-warning {{comparison > result unused}} \ > + // expected-note {{equality > comparison}} > COMMA_MACRO_2(i1 == i2, f(i1, i2)); > - COMMA_MACRO_3(i1 == i2, f(i1, i2)); // expected-warning {{expression > result unused}} > + COMMA_MACRO_3(i1 == i2, f(i1, i2)); // expected-warning {{comparison > result unused}} \ > + // expected-note {{equality > comparison}} > COMMA_MACRO_4(i1 == i2, f(i1, i2)); > } _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
