On Thu, Jan 19, 2012 at 1:11 AM, Dmitri Gribenko <[email protected]> wrote:
> Attached is a patch that implements enhancement proposed in PR11329.
>
> As suggested by Argyrios, this patch implements:
> * a stack of "CompoundScopeInfo"s, helper functions to push/pop them,
> ActOn{Start,Finish}OfCompoundStatement callbacks;
> * a check if the warning is actually enabled before doing costly checks.
>
> Doing this uncovered a bug in TreeTransfom, Sema::ActOnBlockError was
> not called in all cases. This is also fixed.
>
> I've ran a chromium build again and found 3 false positives (-1
> because one previously affected file now builds with -Wno-empty-body).
I accidentally sent an old version with an error. Here's a correct version.
Please review.
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 148530)
+++ 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 148530)
+++ 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 148530)
+++ 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 148530)
+++ 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 148530)
+++ 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 148530)
+++ 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}}
+
+ 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}}
+}
+
+template <typename T>
+void test_template(int x) {
+ if (x); // expected-warning{{if statement has empty body}} \
+ expected-warning{{if statement has empty body}}
+
+ if (x)
+ EMPTY(x); // no-warning
+
+ while (b() == 0); // expected-warning{{while loop has empty body}} \
+ expected-warning{{while loop has empty body}} \
+ expected-note{{put the semicolon on a separate line to silence this warning}} \
+ 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); // expected-note{{requested here}}
+}
+
Index: test/SemaCXX/if-empty-body.cpp
===================================================================
--- test/SemaCXX/if-empty-body.cpp (revision 148530)
+++ 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 148530)
+++ 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 148530)
+++ 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 148530)
+++ test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp (working copy)
@@ -115,5 +115,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 148530)
+++ 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 148530)
+++ 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 148530)
+++ 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 148530)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -4974,8 +4974,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/ScopeInfo.h
===================================================================
--- include/clang/Sema/ScopeInfo.h (revision 148530)
+++ 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: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 148530)
+++ 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.
@@ -2058,6 +2064,8 @@
StmtResult ActOnNullStmt(SourceLocation SemiLoc,
bool HasLeadingEmptyMacro = false);
+ void ActOnStartOfCompoundStmt();
+ void ActOnFinishOfCompoundStmt();
StmtResult ActOnCompoundStmt(SourceLocation L, SourceLocation R,
MultiStmtArg Elts,
bool isStmtExpr);
@@ -2209,6 +2217,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: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp (revision 148530)
+++ lib/Sema/SemaDeclCXX.cpp (working copy)
@@ -8143,9 +8143,11 @@
CopyAssignOperator->setInvalidDecl();
return;
}
-
+
+ ActOnStartOfCompoundStmt();
StmtResult Body = ActOnCompoundStmt(Loc, Loc, move_arg(Statements),
/*isStmtExpr=*/false);
+ ActOnFinishOfCompoundStmt();
assert(!Body.isInvalid() && "Compound statement creation cannot fail");
CopyAssignOperator->setBody(Body.takeAs<Stmt>());
@@ -8573,9 +8575,11 @@
MoveAssignOperator->setInvalidDecl();
return;
}
-
+
+ ActOnStartOfCompoundStmt();
StmtResult Body = ActOnCompoundStmt(Loc, Loc, move_arg(Statements),
/*isStmtExpr=*/false);
+ ActOnFinishOfCompoundStmt();
assert(!Body.isInvalid() && "Compound statement creation cannot fail");
MoveAssignOperator->setBody(Body.takeAs<Stmt>());
@@ -8773,11 +8777,13 @@
<< CXXCopyConstructor << Context.getTagDeclType(ClassDecl);
CopyConstructor->setInvalidDecl();
} else {
+ ActOnStartOfCompoundStmt();
CopyConstructor->setBody(ActOnCompoundStmt(CopyConstructor->getLocation(),
CopyConstructor->getLocation(),
MultiStmtArg(*this, 0, 0),
/*isStmtExpr=*/false)
.takeAs<Stmt>());
+ ActOnFinishOfCompoundStmt();
CopyConstructor->setImplicitlyDefined(true);
}
@@ -8927,11 +8933,13 @@
<< CXXMoveConstructor << Context.getTagDeclType(ClassDecl);
MoveConstructor->setInvalidDecl();
} else {
+ ActOnStartOfCompoundStmt();
MoveConstructor->setBody(ActOnCompoundStmt(MoveConstructor->getLocation(),
MoveConstructor->getLocation(),
MultiStmtArg(*this, 0, 0),
/*isStmtExpr=*/false)
.takeAs<Stmt>());
+ ActOnFinishOfCompoundStmt();
MoveConstructor->setImplicitlyDefined(true);
}
Index: lib/Sema/TreeTransform.h
===================================================================
--- lib/Sema/TreeTransform.h (revision 148530)
+++ lib/Sema/TreeTransform.h (working copy)
@@ -4945,6 +4945,8 @@
StmtResult
TreeTransform<Derived>::TransformCompoundStmt(CompoundStmt *S,
bool IsStmtExpr) {
+ getSema().ActOnStartOfCompoundStmt();
+
bool SubStmtInvalid = false;
bool SubStmtChanged = false;
ASTOwningVector<Stmt*> Statements(getSema());
@@ -4954,8 +4956,10 @@
if (Result.isInvalid()) {
// Immediately fail if this was a DeclStmt, since it's very
// likely that this will cause problems for future statements.
- if (isa<DeclStmt>(*B))
+ if (isa<DeclStmt>(*B)) {
+ getSema().ActOnFinishOfCompoundStmt();
return StmtError();
+ }
// Otherwise, just keep processing substatements and fail later.
SubStmtInvalid = true;
@@ -4966,17 +4970,23 @@
Statements.push_back(Result.takeAs<Stmt>());
}
- if (SubStmtInvalid)
+ if (SubStmtInvalid) {
+ getSema().ActOnFinishOfCompoundStmt();
return StmtError();
+ }
if (!getDerived().AlwaysRebuild() &&
- !SubStmtChanged)
+ !SubStmtChanged) {
+ getSema().ActOnFinishOfCompoundStmt();
return SemaRef.Owned(S);
+ }
- return getDerived().RebuildCompoundStmt(S->getLBracLoc(),
- move_arg(Statements),
- S->getRBracLoc(),
- IsStmtExpr);
+ StmtResult Result = getDerived().RebuildCompoundStmt(S->getLBracLoc(),
+ move_arg(Statements),
+ S->getRBracLoc(),
+ IsStmtExpr);
+ getSema().ActOnFinishOfCompoundStmt();
+ return Result;
}
template<typename Derived>
@@ -8125,8 +8135,10 @@
if (getDerived().TransformFunctionTypeParams(E->getCaretLocation(),
oldBlock->param_begin(),
oldBlock->param_size(),
- 0, paramTypes, ¶ms))
+ 0, paramTypes, ¶ms)) {
+ getSema().ActOnBlockError(E->getCaretLocation(), /*Scope=*/0);
return ExprError();
+ }
const FunctionType *exprFunctionType = E->getFunctionType();
QualType exprResultType = exprFunctionType->getResultType();
@@ -8147,6 +8159,7 @@
getSema().Diag(E->getCaretLocation(),
diag::err_object_cannot_be_passed_returned_by_value)
<< 0 << blockScope->ReturnType;
+ getSema().ActOnBlockError(E->getCaretLocation(), /*Scope=*/0);
return ExprError();
}
@@ -8171,8 +8184,10 @@
// Transform the body
StmtResult body = getDerived().TransformStmt(E->getBody());
- if (body.isInvalid())
+ if (body.isInvalid()) {
+ getSema().ActOnBlockError(E->getCaretLocation(), /*Scope=*/0);
return ExprError();
+ }
#ifndef NDEBUG
// In builds with assertions, make sure that we captured everything we
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp (revision 148530)
+++ 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,13 @@
DiagnoseUnusedExprResult(Elts[i]);
}
+ // Check for suspicious empty body (null statement) in `for' and `while'
+ // statements.
+ if (NumElts != 0 && 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 +372,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);
@@ -956,6 +963,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)
@@ -982,6 +992,9 @@
DiagnoseUnusedExprResult(Body);
+ if (isa<NullStmt>(Body))
+ getCurCompoundScope()->setHasEmptyLoopBodies();
+
return Owned(new (Context) WhileStmt(Context, ConditionVar, ConditionExpr,
Body, WhileLoc));
}
@@ -1045,6 +1058,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,
@@ -1584,6 +1600,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 148530)
+++ lib/Sema/SemaChecking.cpp (working copy)
@@ -4700,3 +4700,125 @@
}
}
}
+
+//===--- CHECK: Empty statement body (-Wempty-body) ---------------------===//
+
+bool Sema::ShouldDiagnoseEmptyStmtBody(SourceLocation StmtLoc,
+ const NullStmt *Body,
+ bool QuietIfBodyOnSeparateLine) {
+ // 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 148530)
+++ lib/Sema/Sema.cpp (working copy)
@@ -856,6 +856,18 @@
}
}
+void Sema::PushCompoundScope() {
+ getCurFunction()->CompoundScopes.push_back(new CompoundScopeInfo);
+}
+
+void Sema::PopCompoundScope() {
+ FunctionScopeInfo *CurFunction = getCurFunction();
+ assert(!CurFunction->CompoundScopes.empty() && "mismatched push/pop");
+
+ CompoundScopeInfo *Scope = CurFunction->CompoundScopes.pop_back_val();
+ delete Scope;
+}
+
/// \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 148530)
+++ lib/Parse/ParseObjc.cpp (working copy)
@@ -2612,9 +2612,12 @@
StmtResult FnBody(ParseCompoundStatementBody());
// If the function body could not be parsed, make a bogus compoundstmt.
- if (FnBody.isInvalid())
+ if (FnBody.isInvalid()) {
+ Actions.ActOnStartOfCompoundStmt();
FnBody = Actions.ActOnCompoundStmt(BraceLoc, BraceLoc,
MultiStmtArg(Actions), false);
+ Actions.ActOnFinishOfCompoundStmt();
+ }
// Leave the function body scope.
BodyScope.Exit();
Index: lib/Parse/ParseStmt.cpp
===================================================================
--- lib/Parse/ParseStmt.cpp (revision 148530)
+++ 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();
+ Actions.ActOnStartOfCompoundStmt();
+
StmtVector Stmts(Actions);
// "__label__ X, Y, Z;" is the GNU "Local Label" extension. These are
@@ -811,14 +812,21 @@
if (Tok.isNot(tok::r_brace)) {
Diag(Tok, diag::err_expected_rbrace);
Diag(T.getOpenLocation(), diag::note_matching) << "{";
+ Actions.ActOnFinishOfCompoundStmt();
return StmtError();
}
- if (T.consumeClose())
+ if (T.consumeClose()) {
+ Actions.ActOnFinishOfCompoundStmt();
return StmtError();
+ }
- return Actions.ActOnCompoundStmt(T.getOpenLocation(), T.getCloseLocation(),
- move_arg(Stmts), isStmtExpr);
+ StmtResult Result = Actions.ActOnCompoundStmt(T.getOpenLocation(),
+ T.getCloseLocation(),
+ move_arg(Stmts), isStmtExpr);
+ Actions.ActOnFinishOfCompoundStmt();
+
+ return Result;
}
/// ParseParenExprOrCondition:
@@ -1084,10 +1092,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 +1970,12 @@
StmtResult FnBody(ParseCompoundStatementBody());
// If the function body could not be parsed, make a bogus compoundstmt.
- if (FnBody.isInvalid())
+ if (FnBody.isInvalid()) {
+ Actions.ActOnStartOfCompoundStmt();
FnBody = Actions.ActOnCompoundStmt(LBraceLoc, LBraceLoc,
MultiStmtArg(Actions), false);
+ Actions.ActOnFinishOfCompoundStmt();
+ }
BodyScope.Exit();
return Actions.ActOnFinishFunctionBody(Decl, FnBody.take());
@@ -1993,9 +2010,12 @@
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()) {
+ Actions.ActOnStartOfCompoundStmt();
FnBody = Actions.ActOnCompoundStmt(LBraceLoc, LBraceLoc,
MultiStmtArg(Actions), false);
+ Actions.ActOnFinishOfCompoundStmt();
+ }
BodyScope.Exit();
return Actions.ActOnFinishFunctionBody(Decl, FnBody.take());
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
