[PATCH] Be yet more clever about -Wunused-value in macros

[Richard Smith]

On Thu, Feb 7, 2013 at 4:18 PM, Matt Beaumont-Gay <matthewbg at google.com>wrote:

>   OK. I'm not super happy with having to check for being in a macro in two
> places, but I didn't see a nicer way. Any ideas?
>

Maybe factor out the computation into a bool, but I don't see a nicer way
either.


> Hi rsmith,
>
> http://llvm-reviews.chandlerc.com/D380
>
> CHANGE SINCE LAST DIFF
>   http://llvm-reviews.chandlerc.com/D380?vs=905&id=919#toc
>
> Files:
>   lib/Sema/SemaStmt.cpp
>   test/Sema/unused-expr.c
>
> Index: lib/Sema/SemaStmt.cpp
> ===================================================================
> --- lib/Sema/SemaStmt.cpp
> +++ lib/Sema/SemaStmt.cpp
> @@ -158,8 +158,7 @@
>    if (!E)
>      return;
>    SourceLocation ExprLoc = E->IgnoreParens()->getExprLoc();
> -  if (SourceMgr.isInSystemMacro(ExprLoc) ||
> -      SourceMgr.isMacroBodyExpansion(ExprLoc))
> +  if (SourceMgr.isInSystemMacro(ExprLoc))
>      return;
>

I think warn_unused_result should even fire through a macro in a system
header.


>    const Expr *WarnExpr;
> @@ -193,12 +192,16 @@
>        return;
>
>      // If the callee has attribute pure, const, or warn_unused_result,
> warn with
> -    // a more specific message to make it clear what is happening.
> +    // a more specific message to make it clear what is happening. If the
> call
> +    // is written in a macro body, only warn if it has the
> warn_unused_result
> +    // attribute.
>      if (const Decl *FD = CE->getCalleeDecl()) {
>        if (FD->getAttr<WarnUnusedResultAttr>()) {
>          Diag(Loc, diag::warn_unused_result) << R1 << R2;
>          return;
>        }
> +      if (SourceMgr.isMacroBodyExpansion(ExprLoc))
> +        return;
>        if (FD->getAttr<PureAttr>()) {
>          Diag(Loc, diag::warn_unused_call) << R1 << R2 << "pure";
>          return;
> @@ -208,7 +211,14 @@
>          return;
>        }
>      }
> -  } else if (const ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(E)) {
> +  }
> +
> +  // For unused values other than call expressions, do not warn if the
> +  // expression is written in a macro body.
> +  if (SourceMgr.isMacroBodyExpansion(ExprLoc))
> +    return;
>

Maybe write this as an 'else if'?


> +
> +  if (const ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(E)) {
>      if (getLangOpts().ObjCAutoRefCount && ME->isDelegateInitCall()) {
>        Diag(Loc, diag::err_arc_unused_init_message) << R1;
>        return;
> Index: test/Sema/unused-expr.c
> ===================================================================
> --- test/Sema/unused-expr.c
> +++ test/Sema/unused-expr.c
> @@ -132,6 +132,8 @@
>  #define M3(a) (t3(a), fn2())
>  #define M4(a, b) (foo((a), (b)) ? 0 : t3(a), 1)
>  #define M5(a, b) (foo((a), (b)), 1)
> +#define M6() fn1()
> +#define M7() fn2()
>  void t11(int i, int j) {
>    M1(i, j);  // no warning
>    NOP((long)foo(i, j)); // expected-warning {{expression result unused}}
> @@ -143,10 +145,14 @@
>    NOP((foo(i, j) ? 0 : t3(i), 1)); // expected-warning {{expression
> result unused}}
>    M5(i, j); // no warning
>    NOP((foo(i, j), 1)); // expected-warning {{expression result unused}}
> +  M6(); // expected-warning {{ignoring return value}}
> +  M7(); // no warning
>  }
>  #undef NOP
>  #undef M1
>  #undef M2
>  #undef M3
>  #undef M4
>  #undef M5
> +#undef M6
> +#undef M7
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130207/6d359e6b/attachment.html>