From 978cca9f49a1c0e3a01096017c9a34726866fbf3 Mon Sep 17 00:00:00 2001 From: Matt Beaumont-Gay Date: Thu, 17 Jan 2013 02:06:08 +0000 Subject: [PATCH] Suppress all -Wunused-value warnings from macro body expansions. This is inspired by a number of false positives in real code, including PR14968. I've added test cases reduced from these false positives 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. This also removes my previous tweak from r166522/r166534, so that 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. llvm-svn: 172696 --- clang/lib/AST/Expr.cpp | 4 --- clang/lib/Sema/SemaStmt.cpp | 7 +++-- clang/test/Misc/caret-diags-macros.c | 34 +++++++++++----------- clang/test/Preprocessor/pragma_microsoft.c | 5 ++-- clang/test/Sema/unused-expr.c | 23 +++++++++++++-- 5 files changed, 45 insertions(+), 28 deletions(-) diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 45f61fa5c210..4796257c92dc 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -2100,10 +2100,6 @@ bool Expr::isUnusedResultAWarning(const Expr *&WarnE, SourceLocation &Loc, 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) diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index db314fa50c66..a2daa18d45b2 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -157,12 +157,15 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S) { const Expr *E = dyn_cast_or_null(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 diff --git a/clang/test/Misc/caret-diags-macros.c b/clang/test/Misc/caret-diags-macros.c index 95fc64cf3c26..538431a17a0e 100644 --- a/clang/test/Misc/caret-diags-macros.c +++ b/clang/test/Misc/caret-diags-macros.c @@ -10,15 +10,15 @@ void foo() { // 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 @@ int y = Y; // 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) diff --git a/clang/test/Preprocessor/pragma_microsoft.c b/clang/test/Preprocessor/pragma_microsoft.c index 156d05243b17..c0ddf74340ce 100644 --- a/clang/test/Preprocessor/pragma_microsoft.c +++ b/clang/test/Preprocessor/pragma_microsoft.c @@ -26,7 +26,7 @@ __pragma(comment(linker," bar=" BAR)) #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 @@ void f() // 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}} } diff --git a/clang/test/Sema/unused-expr.c b/clang/test/Sema/unused-expr.c index aa81febdbbde..0786ede6763f 100644 --- a/clang/test/Sema/unused-expr.c +++ b/clang/test/Sema/unused-expr.c @@ -123,13 +123,30 @@ void f(int i, ...) { // 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