thakis created this revision.
thakis added a reviewer: rsmith.
thakis added a subscriber: cfe-commits.
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. In MS mode, 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
Files:
lib/Sema/SemaStmt.cpp
test/Sema/unused-expr.c
Index: test/Sema/unused-expr.c
===================================================================
--- test/Sema/unused-expr.c
+++ test/Sema/unused-expr.c
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s -Wno-unreachable-code
+// RUN: %clang_cc1 -DMS_COMPAT -fms-compatibility -fsyntax-only -verify %s
-Wno-unreachable-code
int foo(int X, int Y);
@@ -156,3 +157,13 @@
#undef M5
#undef M6
#undef M7
+
+#define UNREFERENCED_PARAMETER(x) (x)
+
+void unused_parm(int a) {
+#ifdef MS_COMPAT
+ UNREFERENCED_PARAMETER(a);
+#else
+ UNREFERENCED_PARAMETER(a); // expected-warning {{expression result unused}}
+#endif
+}
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -195,7 +195,7 @@
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,16 @@
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 (getLangOpts().MSVCCompat && 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.
Index: test/Sema/unused-expr.c
===================================================================
--- test/Sema/unused-expr.c
+++ test/Sema/unused-expr.c
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s -Wno-unreachable-code
+// RUN: %clang_cc1 -DMS_COMPAT -fms-compatibility -fsyntax-only -verify %s -Wno-unreachable-code
int foo(int X, int Y);
@@ -156,3 +157,13 @@
#undef M5
#undef M6
#undef M7
+
+#define UNREFERENCED_PARAMETER(x) (x)
+
+void unused_parm(int a) {
+#ifdef MS_COMPAT
+ UNREFERENCED_PARAMETER(a);
+#else
+ UNREFERENCED_PARAMETER(a); // expected-warning {{expression result unused}}
+#endif
+}
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -195,7 +195,7 @@
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,16 @@
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 (getLangOpts().MSVCCompat && 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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
