r251441 - Tweak how -Wunused-value interacts with macros

[Nico Weber via cfe-commits]

Author: nico
Date: Tue Oct 27 14:47:40 2015
New Revision: 251441

URL: http://llvm.org/viewvc/llvm-project?rev=251441&view=rev
Log:
Tweak how -Wunused-value interacts with macros

1. Make the warning more strict in C mode. r172696 added code to suppress
   warnings from macro expansions in system headers, which checks
   `SourceMgr.isMacroBodyExpansion(E->IgnoreParens()->getExprLoc())`. Consider
   this snippet:

   #define FOO(x) (x)
   void f(int a) {
     FOO(a);
   }

   In C, the line `FOO(a)` is an `ImplicitCastExpr(ParenExpr(DeclRefExpr))`,
   while it's just a `ParenExpr(DeclRefExpr)` in C++. So in C++,
   `E->IgnoreParens()` returns the `DeclRefExpr` and the check tests the
   SourceLoc of `a`. In C, the `ImplicitCastExpr` has the effect of checking the
   SourceLoc of `FOO`, which is a macro body expansion, which causes the
   diagnostic to be skipped. It looks unintentional that clang does different
   things for C and C++ here, so use `IgnoreParenImpCasts` instead of
   `IgnoreParens` here. This has the effect of the warning firing more often
   than previously in C code – it now fires as often as it fires in C++ code.

2. Suppress the warning if it would warn on `UNREFERENCED_PARAMETER`.
   `UNREFERENCED_PARAMETER` is a commonly used macro on Windows and it happens
   to uselessly trigger -Wunused-value. As discussed in the thread
   "rfc: winnt.h's UNREFERENCED_PARAMETER() vs clang's -Wunused-value" on
   cfe-dev, fix this by special-casing this specific macro. (This costs a string
   comparison and some fast-path lexing per warning, but the warning is emitted
   rarely. It fires once in Windows.h itself, so this code runs at least once
   per TU including Windows.h, but it doesn't run hundreds of times.)

http://reviews.llvm.org/D13969

Modified:
    cfe/trunk/lib/Sema/SemaStmt.cpp
    cfe/trunk/test/Sema/unused-expr.c

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=251441&r1=251440&r2=251441&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Tue Oct 27 14:47:40 2015
@@ -195,7 +195,7 @@ void Sema::DiagnoseUnusedExprResult(cons
   if (isUnevaluatedContext())
     return;
 
-  SourceLocation ExprLoc = E->IgnoreParens()->getExprLoc();
+  SourceLocation ExprLoc = E->IgnoreParenImpCasts()->getExprLoc();
   // In most cases, we don't want to warn if the expression is written in a
   // macro body, or if the macro comes from a system header. If the offending
   // expression is a call to a function with the warn_unused_result attribute,
@@ -218,6 +218,15 @@ void Sema::DiagnoseUnusedExprResult(cons
   if (isa<StmtExpr>(E) && Loc.isMacroID())
     return;
 
+  // Check if this is the UNREFERENCED_PARAMETER from the Microsoft headers.
+  // That macro is frequently used to suppress "unused parameter" warnings,
+  // but its implementation makes clang's -Wunused-value fire.  Prevent this.
+  if (isa<ParenExpr>(E->IgnoreImpCasts()) && Loc.isMacroID()) {
+    SourceLocation SpellLoc = Loc;
+    if (findMacroSpelling(SpellLoc, "UNREFERENCED_PARAMETER"))
+      return;
+  }
+
   // Okay, we have an unused result.  Depending on what the base expression is,
   // we might want to make a more specific diagnostic.  Check for one of these
   // cases now.

Modified: cfe/trunk/test/Sema/unused-expr.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/unused-expr.c?rev=251441&r1=251440&r2=251441&view=diff
==============================================================================
--- cfe/trunk/test/Sema/unused-expr.c (original)
+++ cfe/trunk/test/Sema/unused-expr.c Tue Oct 27 14:47:40 2015
@@ -156,3 +156,11 @@ void t11(int i, int j) {
 #undef M5
 #undef M6
 #undef M7
+
+#define UNREFERENCED_PARAMETER(x) (x)
+
+void unused_parm(int a) {
+  // Don't warn if the warning is introduced by a macro that's spelled
+  // UNREFERENCED_PARAMETER, as that's a commonly used macro in Windows headers.
+  UNREFERENCED_PARAMETER(a);
+}