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

[Matt Beaumont-Gay]

Hi rsmith,

My last patch for -Wunused-value and macros broke some no-compile tests. For instance, from gtest:

#define RUN_ALL_TESTS() \
  (::testing::UnitTest::GetInstance()->Run())

The Run() method here is declared with __attribute__((warn_unused_result)), and there's a test which asserts that this doesn't compile (with -Werror):

void f() {
  RUN_ALL_TESTS();
}

This patch carves out an exception from the "don't warn about unused values in macro bodies" rule: if the entirety of the macro (mod parentheses) is a call expression, we potentially should warn.

http://llvm-reviews.chandlerc.com/D380

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,21 @@
   if (!E)
     return;
   SourceLocation ExprLoc = E->IgnoreParens()->getExprLoc();
-  if (SourceMgr.isInSystemMacro(ExprLoc) ||
-      SourceMgr.isMacroBodyExpansion(ExprLoc))
+  if (SourceMgr.isInSystemMacro(ExprLoc))
+    return;
+  // In general, we don't want to warn about unused values coming from
+  // expressions written in macro bodies. However, we do carve out an exception:
+  // if the expression is a CallExpr, and it is the entire body of the macro
+  // (mod parentheses), the macro isn't doing anything clever and we should
+  // still potentially warn.
+  if (SourceMgr.isMacroBodyExpansion(ExprLoc) &&
+      !(isa<CallExpr>(E->IgnoreParens()) &&
+        E->getLocStart().isMacroID() &&
+        Lexer::isAtStartOfMacroExpansion(E->getLocStart(), SourceMgr,
+                                         getLangOpts()) &&
+        E->getLocEnd().isMacroID() &&
+        Lexer::isAtEndOfMacroExpansion(E->getLocEnd(), SourceMgr,
+                                       getLangOpts())))
     return;
 
   const Expr *WarnExpr;
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() (fn1())
 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(); // expected-warning {{ignoring return value}}
 }
 #undef NOP
 #undef M1
 #undef M2
 #undef M3
 #undef M4
 #undef M5
+#undef M6
+#undef M7
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D380.1.patch
Type: text/x-patch
Size: 2083 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130206/60c85485/attachment.bin>