On Wed, Jan 25, 2012 at 9:56 PM, Argyrios Kyrtzidis <[email protected]> wrote:
> I don't think it make sense to warn on template instantiations, since these
> are syntactic warnings. For template instantiations they would only
> contribute noise [...]
Agreed, see attached patch.
Dmitri
--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <[email protected]>*/
Index: test/Analysis/PR9741.cpp
===================================================================
--- test/Analysis/PR9741.cpp (revision 148970)
+++ test/Analysis/PR9741.cpp (working copy)
@@ -4,5 +4,5 @@
int a[] = { 1, 2, 3 };
unsigned int u = 0;
for (auto x : a)
- ;
+ ; // expected-warning{{range-based for loop has empty body}}
}
Index: test/Analysis/misc-ps.m
===================================================================
--- test/Analysis/misc-ps.m (revision 148970)
+++ test/Analysis/misc-ps.m (working copy)
@@ -1248,7 +1248,7 @@
struct s { char *bar[10]; } baz[2] = { 0 };
unsigned i = 0;
for (i = 0;
- (* ({ while(0); ({ &baz[0]; }); })).bar[0] != 0;
+ (* ({ while(0); ({ &baz[0]; }); })).bar[0] != 0; // expected-warning {{while loop has empty body}} expected-note {{put the semicolon on a separate line to silence this warning}}
++i) {}
}
Index: test/Sema/statements.c
===================================================================
--- test/Sema/statements.c (revision 148970)
+++ test/Sema/statements.c (working copy)
@@ -36,7 +36,7 @@
// PR6034
void test11(int bit) {
- switch (bit)
+ switch (bit) // expected-warning {{switch statement has empty body}}
switch (env->fpscr) // expected-error {{use of undeclared identifier 'env'}}
{
}
Index: test/Sema/switch.c
===================================================================
--- test/Sema/switch.c (revision 148970)
+++ test/Sema/switch.c (working copy)
@@ -24,7 +24,7 @@
void test3(void) {
// empty switch;
- switch (0); // expected-warning {{no case matching constant switch condition '0'}}
+ switch (0); // expected-warning {{no case matching constant switch condition '0'}} expected-warning {{switch statement has empty body}}
}
extern int g();
Index: test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p4.cpp
===================================================================
--- test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p4.cpp (revision 148970)
+++ test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p4.cpp (working copy)
@@ -24,7 +24,7 @@
new (auto) (0.0);
int arr[] = {1, 2, 3};
- for (auto i : arr) {
+ for (auto i : arr) { // expected-warning {{range-based for loop has empty body}}
}
}
Index: test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
===================================================================
--- test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp (revision 148970)
+++ test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp (working copy)
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -Wno-empty-body -fsyntax-only -verify %s
namespace std {
template<typename T>
Index: test/SemaCXX/warn-empty-body.cpp
===================================================================
--- test/SemaCXX/warn-empty-body.cpp (revision 0)
+++ test/SemaCXX/warn-empty-body.cpp (revision 0)
@@ -0,0 +1,265 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+void a(int i);
+int b();
+int c();
+
+void test1(int x, int y) {
+ while(true) {
+ if (x); // expected-warning {{if statement has empty body}}
+
+ int i;
+ // PR11329
+ for (i = 0; i < x; i++); { // expected-warning{{for loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+ a(i);
+ b();
+ }
+
+ for (i = 0; i < x; i++); // expected-warning{{for loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+ {
+ a(i);
+ }
+
+ for (i = 0;
+ i < x;
+ i++); // expected-warning{{for loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+ {
+ a(i);
+ }
+
+ int arr[3] = { 1, 2, 3 };
+ for (int j : arr); // expected-warning{{range-based for loop has empty body}}
+ a(i);
+
+ for (int j : arr)
+ {} // expected-warning{{range-based for loop has empty body}}
+
+ for (int j :
+ arr); // expected-warning{{range-based for loop has empty body}}
+ a(i);
+
+ while (b() == 0); // expected-warning{{while loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+ a(i);
+
+ while (b() == 0); { // expected-warning{{while loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+ a(i);
+ }
+
+ while (b() == 0); // expected-warning{{while loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+ {
+ a(i);
+ }
+
+ while (b() == 0 ||
+ c() == 0); // expected-warning{{while loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+ {
+ a(i);
+ }
+
+ switch(x) // no-warning
+ {
+ switch(y); // expected-warning{{switch statement has empty body}}
+ {
+ case 0:
+ a(10);
+ break;
+ default:
+ a(20);
+ break;
+ }
+ }
+ }
+}
+
+/// There should be no warning in `for' and `while' when null statement is
+/// placed on its own line. Range-based `for' and `switch' should warn.
+void test2(int x, int y) {
+ int i;
+ for (i = 0; i < x; i++) // no-warning
+ ; // no-warning
+
+ for (i = 0;
+ i < x;
+ i++) // no-warning
+ ; // no-warning
+
+ int arr[3] = { 1, 2, 3 };
+ for (int j : arr)
+ ; // expected-warning{{range-based for loop has empty body}}
+
+ while (b() == 0) // no-warning
+ ; // no-warning
+
+ while (b() == 0 ||
+ c() == 0) // no-warning
+ ; // no-warning
+
+ switch(x)
+ {
+ switch(y)
+ ; // expected-warning{{switch statement has empty body}}
+ }
+
+ // Last `for' or `while' statement in compound statement shouldn't warn.
+ while(b() == 0); // no-warning
+}
+
+/// There should be no warning for a null statement resulting from an empty macro.
+#define EMPTY(a)
+void test3(int x, int y) {
+ if (x)
+ EMPTY(x); // no-warning
+
+ int i;
+ for (i = 0; i < x; i++) EMPTY(i); // no-warning
+
+ for (i = 0;
+ i < x;
+ i++) EMPTY(i); // no-warning
+
+ int arr[3] = { 1, 2, 3 };
+ for (int j : arr) EMPTY(j); // no-warning
+
+ for (int j :
+ arr) EMPTY(j); // no-warning
+
+ while (b() == 0) EMPTY(i); // no-warning
+
+ while (b() == 0 ||
+ c() == 0) EMPTY(i); // no-warning
+
+ switch (x) {
+ switch (y)
+ EMPTY(i); // no-warning
+ }
+}
+
+/// There should be no warning for a common for/while idiom when it is obvious
+/// from indentation that next statement wasn't meant to be a body.
+void test4(int x, int y) {
+ int i;
+ for (i = 0; i < x; i++); // expected-warning{{for loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+ a(i);
+
+ for (i = 0; i < x; i++); // no-warning
+ a(i);
+
+ for (i = 0;
+ i < x;
+ i++); // expected-warning{{for loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+ a(i);
+
+ for (i = 0;
+ i < x;
+ i++); // no-warning
+ a(i);
+
+ while (b() == 0); // expected-warning{{while loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+ a(i);
+
+ while (b() == 0); // no-warning
+ a(i);
+
+ while (b() == 0 ||
+ c() == 0); // expected-warning{{while loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+ a(i);
+
+ while (b() == 0 ||
+ c() == 0); // no-warning
+ a(i);
+
+}
+
+/// There should be no warning for a statement with a non-null body.
+void test5(int x, int y) {
+ if (x) {} // no-warning
+
+ if (x)
+ a(x); // no-warning
+
+ int i;
+ for (i = 0; i < x; i++) // no-warning
+ a(i); // no-warning
+
+ for (i = 0; i < x; i++) { // no-warning
+ a(i); // no-warning
+ }
+
+ for (i = 0;
+ i < x;
+ i++) // no-warning
+ a(i); // no-warning
+
+ int arr[3] = { 1, 2, 3 };
+ for (int j : arr) // no-warning
+ a(j);
+
+ for (int j :
+ arr) // no-warning
+ a(j);
+
+ while (b() == 0) // no-warning
+ a(i); // no-warning
+
+ while (b() == 0) { // no-warning
+ a(i); // no-warning
+ }
+
+ while (b() == 0 ||
+ c() == 0) // no-warning
+ a(i); // no-warning
+
+ while (b() == 0 ||
+ c() == 0) { // no-warning
+ a(i); // no-warning
+ }
+
+ switch(x) // no-warning
+ {
+ switch(y) // no-warning
+ {
+ case 0:
+ a(10);
+ break;
+ default:
+ a(20);
+ break;
+ }
+ }
+}
+
+void test_errors(int x) {
+ if (1)
+ aa; // expected-error{{use of undeclared identifier}}
+ // no empty body warning.
+
+ int i;
+ for (i = 0; i < x; i++)
+ bb; // expected-error{{use of undeclared identifier}}
+
+ int arr[3] = { 1, 2, 3 };
+ for (int j : arr)
+ cc; // expected-error{{use of undeclared identifier}}
+
+ while (b() == 0)
+ dd; // expected-error{{use of undeclared identifier}}
+}
+
+// Warnings for statements in templates shouldn't be duplicated for all
+// instantiations.
+template <typename T>
+void test_template(int x) {
+ if (x); // expected-warning{{if statement has empty body}}
+
+ if (x)
+ EMPTY(x); // no-warning
+
+ while (b() == 0); // expected-warning{{while loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+ a(x);
+}
+
+void test_template_inst(int x) {
+ test_template<int>(x);
+ test_template<double>(x);
+}
+
Index: test/SemaCXX/if-empty-body.cpp
===================================================================
--- test/SemaCXX/if-empty-body.cpp (revision 148970)
+++ test/SemaCXX/if-empty-body.cpp (working copy)
@@ -1,35 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-
-void f1(int a) {
- if (a); // expected-warning {{if statement has empty body}}
-}
-
-void f2(int a) {
- if (a) {}
-}
-
-void f3() {
- if (1)
- xx; // expected-error {{use of undeclared identifier}}
- return; // no empty body warning.
-}
-
-// Don't warn about an empty body if is expanded from a macro.
-void f4(int i) {
- #define BODY(x)
- if (i == i) // expected-warning{{self-comparison always evaluates to true}}
- BODY(0);
- #undef BODY
-}
-
-template <typename T>
-void tf() {
- #define BODY(x)
- if (0)
- BODY(0);
- #undef BODY
-}
-
-void f5() {
- tf<int>();
-}
Index: test/SemaCXX/for-range-no-std.cpp
===================================================================
--- test/SemaCXX/for-range-no-std.cpp (revision 148970)
+++ test/SemaCXX/for-range-no-std.cpp (working copy)
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++98 -Wno-c++11-extensions
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -Wno-empty-body
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++98 -Wno-c++11-extensions -Wno-empty-body
struct S {
int *begin();
Index: test/SemaCXX/PR10458.cpp
===================================================================
--- test/SemaCXX/PR10458.cpp (revision 148970)
+++ test/SemaCXX/PR10458.cpp (working copy)
@@ -1,7 +1,10 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++98
+void g(int);
+
void f() {
int arr[] = { 1, 2, 3 };
for (auto &i : arr) { // expected-warning {{'auto' type specifier is a C++11 extension}} expected-warning {{range-based for loop is a C++11 extension}}
+ g(i);
}
}
Index: test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
===================================================================
--- test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp (revision 148970)
+++ test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp (working copy)
@@ -120,5 +120,5 @@
static_assert(same_type<decltype(l), std::initializer_list<int>>::value, "");
auto bl = {1, 2.0}; // expected-error {{cannot deduce}}
- for (int i : {1, 2, 3, 4}) {}
+ for (int i : {1, 2, 3, 4}) {} // expected-warning {{range-based for loop has empty body}}
}
Index: test/SemaCXX/for-range-unused.cpp
===================================================================
--- test/SemaCXX/for-range-unused.cpp (revision 148970)
+++ test/SemaCXX/for-range-unused.cpp (working copy)
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -Wunused
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -Wunused -Wno-empty-body
// PR9968: We used to warn that __range is unused in a dependent for-range.
Index: test/SemaCXX/cxx98-compat.cpp
===================================================================
--- test/SemaCXX/cxx98-compat.cpp (revision 148970)
+++ test/SemaCXX/cxx98-compat.cpp (working copy)
@@ -65,9 +65,11 @@
auto f() -> int; // expected-warning {{trailing return types are incompatible with C++98}}
+void g(int);
void RangeFor() {
int xs[] = {1, 2, 3};
for (int &a : xs) { // expected-warning {{range-based for loop is incompatible with C++98}}
+ g(a);
}
}
Index: test/SemaCXX/condition.cpp
===================================================================
--- test/SemaCXX/condition.cpp (revision 148970)
+++ test/SemaCXX/condition.cpp (working copy)
@@ -20,7 +20,7 @@
while (struct S {} *x=0) ; // expected-error {{types may not be defined in conditions}}
while (struct {} *x=0) ; // expected-error {{types may not be defined in conditions}}
switch (enum {E} x=0) ; // expected-error {{types may not be defined in conditions}} expected-error {{cannot initialize}} \
- // expected-warning{{enumeration value 'E' not handled in switch}}
+ // expected-warning{{enumeration value 'E' not handled in switch}} expected-warning {{switch statement has empty body}}
if (int x=0) { // expected-note 2 {{previous definition is here}}
int x; // expected-error {{redefinition of 'x'}}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 148970)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -5009,8 +5009,20 @@
"switch condition type %0 requires explicit conversion to %1">;
def err_switch_incomplete_class_type : Error<
"switch condition has incomplete class type %0">;
+
def warn_empty_if_body : Warning<
"if statement has empty body">, InGroup<EmptyBody>;
+def warn_empty_for_body : Warning<
+ "for loop has empty body">, InGroup<EmptyBody>;
+def warn_empty_range_based_for_body : Warning<
+ "range-based for loop has empty body">, InGroup<EmptyBody>;
+def warn_empty_while_body : Warning<
+ "while loop has empty body">, InGroup<EmptyBody>;
+def warn_empty_switch_body : Warning<
+ "switch statement has empty body">, InGroup<EmptyBody>;
+def note_empty_body_on_separate_line : Note<
+ "put the semicolon on a separate line to silence this warning">,
+ InGroup<EmptyBody>;
def err_va_start_used_in_non_variadic_function : Error<
"'va_start' used in function with fixed args">;
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 148970)
+++ include/clang/Sema/Sema.h (working copy)
@@ -162,6 +162,7 @@
namespace sema {
class AccessedEntity;
class BlockScopeInfo;
+ class CompoundScopeInfo;
class DelayedDiagnostic;
class FunctionScopeInfo;
class LambdaScopeInfo;
@@ -735,6 +736,11 @@
return FunctionScopes.back();
}
+ void PushCompoundScope();
+ void PopCompoundScope();
+
+ sema::CompoundScopeInfo &getCurCompoundScope() const;
+
bool hasAnyUnrecoverableErrorsInThisFunction() const;
/// \brief Retrieve the current block, if any.
@@ -2062,9 +2068,28 @@
StmtResult ActOnNullStmt(SourceLocation SemiLoc,
bool HasLeadingEmptyMacro = false);
+
+ void ActOnStartOfCompoundStmt();
+ void ActOnFinishOfCompoundStmt();
StmtResult ActOnCompoundStmt(SourceLocation L, SourceLocation R,
MultiStmtArg Elts,
bool isStmtExpr);
+
+ /// \brief A RAII object to enter scope of a compound statement.
+ class CompoundScopeRAII {
+ public:
+ CompoundScopeRAII(Sema &S): S(S) {
+ S.ActOnStartOfCompoundStmt();
+ }
+
+ ~CompoundScopeRAII() {
+ S.ActOnFinishOfCompoundStmt();
+ }
+
+ private:
+ Sema &S;
+ };
+
StmtResult ActOnDeclStmt(DeclGroupPtrTy Decl,
SourceLocation StartLoc,
SourceLocation EndLoc);
@@ -2213,6 +2238,30 @@
void DiagnoseUnusedExprResult(const Stmt *S);
void DiagnoseUnusedDecl(const NamedDecl *ND);
+ /// Return true if statement located on \p StmtLoc has a suspicious null
+ /// statement as a \p Body.
+ ///
+ /// If \p QuietIfBodyOnSeparateLine is true, warn only if body is on the same
+ /// line. If \p QuietIfBodyOnSeparateLine is false, always warn.
+ ///
+ /// This helps prevent bugs due to typos, such as:
+ /// if (condition);
+ /// do_stuff();
+ bool ShouldDiagnoseEmptyStmtBody(SourceLocation StmtLoc,
+ const NullStmt *Body,
+ bool QuietIfBodyOnSeparateLine);
+
+ /// Emit \p DiagID if \ref Sema::ShouldDiagnoseEmptyStmtBody returns true.
+ void DiagnoseEmptyStmtBody(SourceLocation StmtLoc,
+ const Stmt *Body,
+ bool QuietIfBodyOnSeparateLine,
+ unsigned DiagID);
+
+ /// Warn if a for/while loop statement \p S, which is followed by
+ /// \p PossibleBody, has a suspicious null statement as a body.
+ void DiagnoseEmptyLoopBody(const Stmt *S,
+ const Stmt *PossibleBody);
+
ParsingDeclState PushParsingDeclaration() {
return DelayedDiagnostics.pushParsingDecl();
}
Index: include/clang/Sema/ScopeInfo.h
===================================================================
--- include/clang/Sema/ScopeInfo.h (revision 148970)
+++ include/clang/Sema/ScopeInfo.h (working copy)
@@ -31,6 +31,22 @@
namespace sema {
+/// \brief Contains information about the compound statement currently being
+/// parsed.
+class CompoundScopeInfo {
+public:
+ CompoundScopeInfo()
+ : HasEmptyLoopBodies(false) { }
+
+ /// \brief Whether this compound stamement contains `for' or `while' loops
+ /// with empty bodies.
+ bool HasEmptyLoopBodies;
+
+ void setHasEmptyLoopBodies() {
+ HasEmptyLoopBodies = true;
+ }
+};
+
class PossiblyUnreachableDiag {
public:
PartialDiagnostic PD;
@@ -78,7 +94,11 @@
/// block, if there is any chance of applying the named return value
/// optimization.
SmallVector<ReturnStmt*, 4> Returns;
-
+
+ /// \brief The stack of currently active compound stamement scopes in the
+ /// function.
+ SmallVector<CompoundScopeInfo, 4> CompoundScopes;
+
/// \brief A list of PartialDiagnostics created but delayed within the
/// current function scope. These diagnostics are vetted for reachability
/// prior to being emitted.
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp (revision 148970)
+++ lib/Sema/SemaDeclCXX.cpp (working copy)
@@ -8160,10 +8160,14 @@
CopyAssignOperator->setInvalidDecl();
return;
}
-
- StmtResult Body = ActOnCompoundStmt(Loc, Loc, move_arg(Statements),
- /*isStmtExpr=*/false);
- assert(!Body.isInvalid() && "Compound statement creation cannot fail");
+
+ StmtResult Body;
+ {
+ CompoundScopeRAII CompoundScope(*this);
+ Body = ActOnCompoundStmt(Loc, Loc, move_arg(Statements),
+ /*isStmtExpr=*/false);
+ assert(!Body.isInvalid() && "Compound statement creation cannot fail");
+ }
CopyAssignOperator->setBody(Body.takeAs<Stmt>());
if (ASTMutationListener *L = getASTMutationListener()) {
@@ -8590,10 +8594,14 @@
MoveAssignOperator->setInvalidDecl();
return;
}
-
- StmtResult Body = ActOnCompoundStmt(Loc, Loc, move_arg(Statements),
- /*isStmtExpr=*/false);
- assert(!Body.isInvalid() && "Compound statement creation cannot fail");
+
+ StmtResult Body;
+ {
+ CompoundScopeRAII CompoundScope(*this);
+ Body = ActOnCompoundStmt(Loc, Loc, move_arg(Statements),
+ /*isStmtExpr=*/false);
+ assert(!Body.isInvalid() && "Compound statement creation cannot fail");
+ }
MoveAssignOperator->setBody(Body.takeAs<Stmt>());
if (ASTMutationListener *L = getASTMutationListener()) {
@@ -8792,9 +8800,10 @@
<< CXXCopyConstructor << Context.getTagDeclType(ClassDecl);
CopyConstructor->setInvalidDecl();
} else {
+ Sema::CompoundScopeRAII CompoundScope(*this);
CopyConstructor->setBody(ActOnCompoundStmt(CopyConstructor->getLocation(),
CopyConstructor->getLocation(),
- MultiStmtArg(*this, 0, 0),
+ MultiStmtArg(*this, 0, 0),
/*isStmtExpr=*/false)
.takeAs<Stmt>());
CopyConstructor->setImplicitlyDefined(true);
@@ -8946,9 +8955,10 @@
<< CXXMoveConstructor << Context.getTagDeclType(ClassDecl);
MoveConstructor->setInvalidDecl();
} else {
+ Sema::CompoundScopeRAII CompoundScope(*this);
MoveConstructor->setBody(ActOnCompoundStmt(MoveConstructor->getLocation(),
MoveConstructor->getLocation(),
- MultiStmtArg(*this, 0, 0),
+ MultiStmtArg(*this, 0, 0),
/*isStmtExpr=*/false)
.takeAs<Stmt>());
MoveConstructor->setImplicitlyDefined(true);
Index: lib/Sema/TreeTransform.h
===================================================================
--- lib/Sema/TreeTransform.h (revision 148970)
+++ lib/Sema/TreeTransform.h (working copy)
@@ -4952,6 +4952,8 @@
StmtResult
TreeTransform<Derived>::TransformCompoundStmt(CompoundStmt *S,
bool IsStmtExpr) {
+ Sema::CompoundScopeRAII CompoundScope(getSema());
+
bool SubStmtInvalid = false;
bool SubStmtChanged = false;
ASTOwningVector<Stmt*> Statements(getSema());
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp (revision 148970)
+++ lib/Sema/SemaStmt.cpp (working copy)
@@ -225,6 +225,18 @@
DiagRuntimeBehavior(Loc, 0, PDiag(DiagID) << R1 << R2);
}
+void Sema::ActOnStartOfCompoundStmt() {
+ PushCompoundScope();
+}
+
+void Sema::ActOnFinishOfCompoundStmt() {
+ PopCompoundScope();
+}
+
+sema::CompoundScopeInfo &Sema::getCurCompoundScope() const {
+ return getCurFunction()->CompoundScopes.back();
+}
+
StmtResult
Sema::ActOnCompoundStmt(SourceLocation L, SourceLocation R,
MultiStmtArg elts, bool isStmtExpr) {
@@ -257,6 +269,14 @@
DiagnoseUnusedExprResult(Elts[i]);
}
+ // Check for suspicious empty body (null statement) in `for' and `while'
+ // statements.
+ if (NumElts != 0 && !CurrentInstantiationScope &&
+ getCurCompoundScope().HasEmptyLoopBodies) {
+ for (unsigned i = 0; i != NumElts - 1; ++i)
+ DiagnoseEmptyLoopBody(Elts[i], Elts[i + 1]);
+ }
+
return Owned(new (Context) CompoundStmt(Context, Elts, NumElts, L, R));
}
@@ -353,21 +373,9 @@
DiagnoseUnusedExprResult(thenStmt);
- // Warn if the if block has a null body without an else value.
- // this helps prevent bugs due to typos, such as
- // if (condition);
- // do_stuff();
- //
if (!elseStmt) {
- if (NullStmt* stmt = dyn_cast<NullStmt>(thenStmt))
- // But do not warn if the body is a macro that expands to nothing, e.g:
- //
- // #define CALL(x)
- // if (condition)
- // CALL(0);
- //
- if (!stmt->hasLeadingEmptyMacro())
- Diag(stmt->getSemiLoc(), diag::warn_empty_if_body);
+ DiagnoseEmptyStmtBody(SourceLocation(), thenStmt, false,
+ diag::warn_empty_if_body);
}
DiagnoseUnusedExprResult(elseStmt);
@@ -958,6 +966,9 @@
}
}
+ DiagnoseEmptyStmtBody(SourceLocation(), BodyStmt, false,
+ diag::warn_empty_switch_body);
+
// FIXME: If the case list was broken is some way, we don't have a good system
// to patch it up. Instead, just return the whole substmt as broken.
if (CaseListIsErroneous)
@@ -984,6 +995,9 @@
DiagnoseUnusedExprResult(Body);
+ if (isa<NullStmt>(Body))
+ getCurCompoundScope().setHasEmptyLoopBodies();
+
return Owned(new (Context) WhileStmt(Context, ConditionVar, ConditionExpr,
Body, WhileLoc));
}
@@ -1047,6 +1061,9 @@
DiagnoseUnusedExprResult(Third);
DiagnoseUnusedExprResult(Body);
+ if (isa<NullStmt>(Body))
+ getCurCompoundScope().setHasEmptyLoopBodies();
+
return Owned(new (Context) ForStmt(Context, First,
SecondResult.take(), ConditionVar,
Third, Body, ForLoc, LParenLoc,
@@ -1587,6 +1604,17 @@
return StmtError();
cast<CXXForRangeStmt>(S)->setBody(B);
+
+ DiagnoseEmptyStmtBody(SourceLocation(), B, false,
+ diag::warn_empty_range_based_for_body);
+
+ // Range-based for with empty body doesn't make sense in any case.
+ if (CompoundStmt *Body = dyn_cast<CompoundStmt>(B)) {
+ if (Body->body_empty())
+ Diag(Body->getLBracLoc(), diag::warn_empty_range_based_for_body)
+ << Body->getSourceRange();
+ }
+
return S;
}
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp (revision 148970)
+++ lib/Sema/SemaChecking.cpp (working copy)
@@ -4710,3 +4710,130 @@
}
}
}
+
+//===--- CHECK: Empty statement body (-Wempty-body) ---------------------===//
+
+bool Sema::ShouldDiagnoseEmptyStmtBody(SourceLocation StmtLoc,
+ const NullStmt *Body,
+ bool QuietIfBodyOnSeparateLine) {
+ // Since this is a syntactic check, don't emit diagnostic for template
+ // instantiations, this just adds noise.
+ if (CurrentInstantiationScope)
+ return false;
+
+ // Do not warn if the body is a macro that expands to nothing, e.g:
+ //
+ // #define CALL(x)
+ // if (condition)
+ // CALL(0);
+ //
+ if (Body->hasLeadingEmptyMacro())
+ return false;
+
+ if (QuietIfBodyOnSeparateLine) {
+ // Get line numbers of statement and body.
+ bool StmtLineInvalid;
+ unsigned StmtLine = SourceMgr.getSpellingLineNumber(StmtLoc,
+ &StmtLineInvalid);
+ if (StmtLineInvalid)
+ return false;
+
+ bool BodyLineInvalid;
+ unsigned BodyLine = SourceMgr.getSpellingLineNumber(Body->getSemiLoc(),
+ &BodyLineInvalid);
+ if (BodyLineInvalid)
+ return false;
+
+ // Warn if null statement and body are on the same line.
+ if (StmtLine != BodyLine)
+ return false;
+ }
+
+ return true;
+}
+
+void Sema::DiagnoseEmptyStmtBody(SourceLocation StmtLoc,
+ const Stmt *Body,
+ bool QuietIfBodyOnSeparateLine,
+ unsigned DiagID) {
+ // The body should be a null statement.
+ const NullStmt *NBody = dyn_cast<NullStmt>(Body);
+ if (!NBody)
+ return;
+
+ // Do the usual checks.
+ if (!ShouldDiagnoseEmptyStmtBody(StmtLoc, NBody, QuietIfBodyOnSeparateLine))
+ return;
+
+ Diag(NBody->getSemiLoc(), DiagID);
+ if (QuietIfBodyOnSeparateLine)
+ Diag(NBody->getSemiLoc(), diag::note_empty_body_on_separate_line);
+}
+
+void Sema::DiagnoseEmptyLoopBody(const Stmt *S,
+ const Stmt *PossibleBody) {
+ SourceLocation StmtLoc;
+ const Stmt *Body;
+ unsigned DiagID;
+ if (const ForStmt *FS = dyn_cast<ForStmt>(S)) {
+ StmtLoc = FS->getRParenLoc();
+ Body = FS->getBody();
+ DiagID = diag::warn_empty_for_body;
+ } else if (const WhileStmt *WS = dyn_cast<WhileStmt>(S)) {
+ StmtLoc = WS->getCond()->getSourceRange().getEnd();
+ Body = WS->getBody();
+ DiagID = diag::warn_empty_while_body;
+ } else
+ return; // Neither `for' nor `while'.
+
+ // The body should be a null statement.
+ const NullStmt *NBody = dyn_cast<NullStmt>(Body);
+ if (!NBody)
+ return;
+
+ // Skip expensive checks if warning is disabled
+ if (Diags.getDiagnosticLevel(DiagID, NBody->getSemiLoc()) ==
+ DiagnosticsEngine::Ignored)
+ return;
+
+ // Do the usual checks.
+ if (!ShouldDiagnoseEmptyStmtBody(StmtLoc, NBody, true))
+ return;
+
+ // `for(...);' and `while(...);' are popular idioms, so in order to keep
+ // noise level low, emit diagnostics only if for/while are followed by a
+ // CompoundStmt, e.g.:
+ // for (int i = 0; i < n; i++);
+ // {
+ // a(i);
+ // }
+ // or if for/while is followed by a statement with more indentation
+ // than for/while itself:
+ // for (int i = 0; i < n; i++);
+ // a(i);
+ bool ProbableTypo = isa<CompoundStmt>(PossibleBody);
+ if (!ProbableTypo) {
+ bool BodyColInvalid;
+ unsigned BodyCol = SourceMgr.getPresumedColumnNumber(
+ PossibleBody->getLocStart(),
+ &BodyColInvalid);
+ if (BodyColInvalid)
+ return;
+
+ bool StmtColInvalid;
+ unsigned StmtCol = SourceMgr.getPresumedColumnNumber(
+ S->getLocStart(),
+ &StmtColInvalid);
+ if (StmtColInvalid)
+ return;
+
+ if (BodyCol > StmtCol)
+ ProbableTypo = true;
+ }
+
+ if (ProbableTypo) {
+ Diag(NBody->getSemiLoc(), DiagID);
+ Diag(NBody->getSemiLoc(), diag::note_empty_body_on_separate_line);
+ }
+}
+
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp (revision 148970)
+++ lib/Sema/Sema.cpp (working copy)
@@ -856,6 +856,17 @@
}
}
+void Sema::PushCompoundScope() {
+ getCurFunction()->CompoundScopes.push_back(CompoundScopeInfo());
+}
+
+void Sema::PopCompoundScope() {
+ FunctionScopeInfo *CurFunction = getCurFunction();
+ assert(!CurFunction->CompoundScopes.empty() && "mismatched push/pop");
+
+ CurFunction->CompoundScopes.pop_back();
+}
+
/// \brief Determine whether any errors occurred within this function/method/
/// block.
bool Sema::hasAnyUnrecoverableErrorsInThisFunction() const {
Index: lib/Parse/ParseObjc.cpp
===================================================================
--- lib/Parse/ParseObjc.cpp (revision 148970)
+++ lib/Parse/ParseObjc.cpp (working copy)
@@ -2612,9 +2612,11 @@
StmtResult FnBody(ParseCompoundStatementBody());
// If the function body could not be parsed, make a bogus compoundstmt.
- if (FnBody.isInvalid())
+ if (FnBody.isInvalid()) {
+ Sema::CompoundScopeRAII CompoundScope(Actions);
FnBody = Actions.ActOnCompoundStmt(BraceLoc, BraceLoc,
MultiStmtArg(Actions), false);
+ }
// Leave the function body scope.
BodyScope.Exit();
Index: lib/Parse/ParseStmt.cpp
===================================================================
--- lib/Parse/ParseStmt.cpp (revision 148970)
+++ lib/Parse/ParseStmt.cpp (working copy)
@@ -700,7 +700,6 @@
return ParseCompoundStatementBody(isStmtExpr);
}
-
/// ParseCompoundStatementBody - Parse a sequence of statements and invoke the
/// ActOnCompoundStmt action. This expects the '{' to be the current token, and
/// consume the '}' at the end of the block. It does not manipulate the scope
@@ -714,6 +713,8 @@
if (T.consumeOpen())
return StmtError();
+ Sema::CompoundScopeRAII CompoundScope(Actions);
+
StmtVector Stmts(Actions);
// "__label__ X, Y, Z;" is the GNU "Local Label" extension. These are
@@ -1084,10 +1085,16 @@
InnerScope.Exit();
SwitchScope.Exit();
- if (Body.isInvalid())
+ if (Body.isInvalid()) {
// FIXME: Remove the case statement list from the Switch statement.
- Body = Actions.ActOnNullStmt(Tok.getLocation());
+ // Put the synthesized null statement on the same line as the end of switch
+ // condition.
+ SourceLocation SynthesizedNullStmtLocation
+ = Cond.get()->getSourceRange().getEnd();
+ Body = Actions.ActOnNullStmt(SynthesizedNullStmtLocation);
+ }
+
return Actions.ActOnFinishSwitchStmt(SwitchLoc, Switch.get(), Body.get());
}
@@ -1956,9 +1963,11 @@
StmtResult FnBody(ParseCompoundStatementBody());
// If the function body could not be parsed, make a bogus compoundstmt.
- if (FnBody.isInvalid())
+ if (FnBody.isInvalid()) {
+ Sema::CompoundScopeRAII CompoundScope(Actions);
FnBody = Actions.ActOnCompoundStmt(LBraceLoc, LBraceLoc,
MultiStmtArg(Actions), false);
+ }
BodyScope.Exit();
return Actions.ActOnFinishFunctionBody(Decl, FnBody.take());
@@ -1993,9 +2002,11 @@
StmtResult FnBody(ParseCXXTryBlockCommon(TryLoc));
// If we failed to parse the try-catch, we just give the function an empty
// compound statement as the body.
- if (FnBody.isInvalid())
+ if (FnBody.isInvalid()) {
+ Sema::CompoundScopeRAII CompoundScope(Actions);
FnBody = Actions.ActOnCompoundStmt(LBraceLoc, LBraceLoc,
MultiStmtArg(Actions), false);
+ }
BodyScope.Exit();
return Actions.ActOnFinishFunctionBody(Decl, FnBody.take());
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
