[cfe-commits] [PATCH] Suppress -Wunused-value warnings from macro bodies
Matt Beaumont-Gay
matthewbg at google.com
Wed Jan 16 17:35:50 PST 2013
Hi rsmith,
This patch makes clang not emit -Wunused-value warnings for expressions in macro bodies (as opposed to macro arguments). This is inspired by a number of false positives in real code. From libpcap:
#define strlcpy(x, y, z) \
(strncpy((x), (y), (z)), \
((z) <= 0 ? 0 : ((x)[(z) - 1] = '\0')), \
strlen((y)))
Ignoring the result of the strlcpy call results in a warning for not using the return value of a function marked 'pure' (strlen). Another false positive looks like:
#define VERIFY_THING(cond, message) ((cond) ? true : Log(message), false)
In a use like:
if (VERIFY_THING(foo, "foo isn't OK"))
VERIFY_THING(bar, "bar isn't OK");
we warn on the second line (pointing at the 'true' in the macro). Finally, just today, PR14968 was filed with this example:
int foo(int a);
#define foo(a) (foo(a), 1);
void bar(int a) {
foo(a);
}
I've added test cases resembling all of these to test/Sema/unused-expr.c, as well as corresponding test cases that pass the offending expressions as arguments to a no-op macro to ensure that we do warn there.
The actual code change is pretty trivial, though I also removed my previous tweak from r166522/r166534 so we warn on unused cast expressions in macro arguments.
There were several test cases that were using -Wunused-value to test general diagnostic emission features; I changed those to use other warnings or warn on a macro argument expression. I stared at the test case for PR14399 for a while with Richard Smith and we believe the new test case exercises the same codepaths as before.
http://llvm-reviews.chandlerc.com/D305
Files:
lib/AST/Expr.cpp
lib/Sema/SemaStmt.cpp
test/Misc/caret-diags-macros.c
test/Preprocessor/pragma_microsoft.c
test/Sema/unused-expr.c
Index: lib/AST/Expr.cpp
===================================================================
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -2100,10 +2100,6 @@
return false;
}
- // Ignore casts within macro expansions.
- if (getExprLoc().isMacroID())
- return CE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
-
// If this is a cast to a constructor conversion, check the operand.
// Otherwise, the result of the cast is unused.
if (CE->getCastKind() == CK_ConstructorConversion)
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -157,12 +157,15 @@
const Expr *E = dyn_cast_or_null<Expr>(S);
if (!E)
return;
+ SourceLocation ExprLoc = E->IgnoreParens()->getExprLoc();
+ if (SourceMgr.isInSystemMacro(ExprLoc) ||
+ SourceMgr.isMacroBodyExpansion(ExprLoc))
+ return;
const Expr *WarnExpr;
SourceLocation Loc;
SourceRange R1, R2;
- if (SourceMgr.isInSystemMacro(E->getExprLoc()) ||
- !E->isUnusedResultAWarning(WarnExpr, Loc, R1, R2, Context))
+ if (!E->isUnusedResultAWarning(WarnExpr, Loc, R1, R2, Context))
return;
// If this is a GNU statement expression expanded from a macro, it is probably
Index: test/Misc/caret-diags-macros.c
===================================================================
--- test/Misc/caret-diags-macros.c
+++ test/Misc/caret-diags-macros.c
@@ -10,15 +10,15 @@
// CHECK: {{.*}}:3:{{[0-9]+}}: note: expanded from macro 'M1'
}
-#define A 1
-#define B A
-#define C B
+#define A(x) x
+#define B(x) A(x)
+#define C(x) B(x)
void bar() {
- C;
- // CHECK: {{.*}}:17:3: warning: expression result unused
- // CHECK: {{.*}}:15:11: note: expanded from macro 'C'
- // CHECK: {{.*}}:14:11: note: expanded from macro 'B'
- // CHECK: {{.*}}:13:11: note: expanded from macro 'A'
+ C(1);
+ // CHECK: {{.*}}:17:5: warning: expression result unused
+ // CHECK: {{.*}}:15:16: note: expanded from macro 'C'
+ // CHECK: {{.*}}:14:16: note: expanded from macro 'B'
+ // CHECK: {{.*}}:13:14: note: expanded from macro 'A'
}
// rdar://7597492
@@ -174,17 +174,17 @@
// PR14399
void iequals(int,int,int);
-void foo_aa()
+void foo_aa(char* s)
{
-#define /* */ BARC(c, /* */b, a, ...) (a+b+/* */c + __VA_ARGS__ +0)
- iequals(__LINE__, BARC(4,3,2,6,8), 8);
+#define /* */ BARC(c, /* */b, a) (a + b ? c : c)
+ iequals(__LINE__, BARC(123, (456 < 345), 789), 8);
}
-// CHECK: {{.*}}:180:21: warning: expression result unused
-// CHECK-NEXT: iequals(__LINE__, BARC(4,3,2,6,8), 8);
-// CHECK-NEXT: {{^ \^~~~~~~~~~~~~~~}}
-// CHECK-NEXT: {{.*}}:179:51: note: expanded from macro 'BARC'
-// CHECK-NEXT: #define /* */ BARC(c, /* */b, a, ...) (a+b+/* */c + __VA_ARGS__ +0)
-// CHECK-NEXT: {{^ ~~~~~~~~~~ \^}}
+// CHECK: {{.*}}:180:21: warning: operator '?:' has lower precedence than '+'
+// CHECK-NEXT: iequals(__LINE__, BARC(123, (456 < 345), 789), 8);
+// CHECK-NEXT: {{^ \^~~~~~~~~~~~~~~~~~~~~~~~~~~}}
+// CHECK-NEXT: {{.*}}:179:41: note: expanded from macro 'BARC'
+// CHECK-NEXT: #define /* */ BARC(c, /* */b, a) (a + b ? c : c)
+// CHECK-NEXT: {{^ ~~~~~ \^}}
#define APPEND2(NUM, SUFF) -1 != NUM ## SUFF
#define APPEND(NUM, SUFF) APPEND2(NUM, SUFF)
Index: test/Preprocessor/pragma_microsoft.c
===================================================================
--- test/Preprocessor/pragma_microsoft.c
+++ test/Preprocessor/pragma_microsoft.c
@@ -26,7 +26,7 @@
#define MACRO_WITH__PRAGMA { \
__pragma(warning(push)); \
__pragma(warning(disable: 10000)); \
- 2+2; \
+ 1 + (2 > 3) ? 4 : 5; \
__pragma(warning(pop)); \
}
@@ -36,7 +36,8 @@
// If we ever actually *support* __pragma(warning(disable: x)),
// this warning should go away.
- MACRO_WITH__PRAGMA // expected-warning {{expression result unused}}
+ MACRO_WITH__PRAGMA // expected-warning {{lower precedence}} \
+ // expected-note 2 {{place parentheses}}
}
Index: test/Sema/unused-expr.c
===================================================================
--- test/Sema/unused-expr.c
+++ test/Sema/unused-expr.c
@@ -123,13 +123,30 @@
// PR8371
int fn5() __attribute__ ((__const));
-// OpenSSL has some macros like this; we shouldn't warn on the cast.
+// Don't warn for unused expressions in macro bodies; however, do warn for
+// unused expressions in macro arguments. Macros below are reduced from code
+// found in the wild.
+#define NOP(a) (a)
#define M1(a, b) (long)foo((a), (b))
-// But, we should still warn on other subexpressions of casts in macros.
#define M2 (long)0;
+#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)
void t11(int i, int j) {
M1(i, j); // no warning
- M2; // expected-warning {{expression result unused}}
+ NOP((long)foo(i, j)); // expected-warning {{expression result unused}}
+ M2; // no warning
+ NOP((long)0); // expected-warning {{expression result unused}}
+ M3(i); // no warning
+ NOP((t3(i), fn2())); // expected-warning {{ignoring return value}}
+ M4(i, j); // no warning
+ 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}}
}
+#undef NOP
#undef M1
#undef M2
+#undef M3
+#undef M4
+#undef M5
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D305.1.patch
Type: text/x-patch
Size: 5536 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130116/239342af/attachment.bin>
More information about the cfe-commits
mailing list