[cfe-commits] [PATCH] Properly check for unused comparisons in arguments to function-like macros

[Matt Beaumont-Gay]

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 <matthewbg at google.com> 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));
>  }