diff options
author | Dmitri Gribenko <gribozavr@gmail.com> | 2012-02-14 22:14:32 +0000 |
---|---|---|
committer | Dmitri Gribenko <gribozavr@gmail.com> | 2012-02-14 22:14:32 +0000 |
commit | 800ddf3dda7b21f652da3d9720374997697f17e6 (patch) | |
tree | c4c8ce89332122705d7a8aaed4a1ebb43508b0b4 | |
parent | 973cf9e8ae0a79c33d4e0f58af0b596ddd96faa1 (diff) | |
download | bcm5719-llvm-800ddf3dda7b21f652da3d9720374997697f17e6.tar.gz bcm5719-llvm-800ddf3dda7b21f652da3d9720374997697f17e6.zip |
Generalize -Wempty-body: warn when statement body is empty (closes: PR11329)
* if, switch, range-based for: warn if semicolon is on the same line.
* for, while: warn if semicolon is on the same line and either next
statement is compound statement or next statement has more
indentation.
Replacing the semicolon with {} or moving the semicolon to the next
line will always silence the warning.
Tests from SemaCXX/if-empty-body.cpp merged into SemaCXX/warn-empty-body.cpp.
llvm-svn: 150515
-rw-r--r-- | clang/include/clang/Basic/DiagnosticSemaKinds.td | 12 | ||||
-rw-r--r-- | clang/include/clang/Sema/ScopeInfo.h | 22 | ||||
-rw-r--r-- | clang/include/clang/Sema/Sema.h | 40 | ||||
-rw-r--r-- | clang/lib/Parse/ParseObjc.cpp | 4 | ||||
-rw-r--r-- | clang/lib/Parse/ParseStmt.cpp | 20 | ||||
-rw-r--r-- | clang/lib/Sema/Sema.cpp | 11 | ||||
-rw-r--r-- | clang/lib/Sema/SemaChecking.cpp | 127 | ||||
-rw-r--r-- | clang/lib/Sema/SemaDeclCXX.cpp | 30 | ||||
-rw-r--r-- | clang/lib/Sema/SemaStmt.cpp | 53 | ||||
-rw-r--r-- | clang/lib/Sema/TreeTransform.h | 2 | ||||
-rw-r--r-- | clang/test/Analysis/misc-ps.m | 2 | ||||
-rw-r--r-- | clang/test/Parser/cxx-condition.cpp | 2 | ||||
-rw-r--r-- | clang/test/Parser/statements.c | 8 | ||||
-rw-r--r-- | clang/test/Sema/default.c | 2 | ||||
-rw-r--r-- | clang/test/Sema/statements.c | 2 | ||||
-rw-r--r-- | clang/test/Sema/switch.c | 4 | ||||
-rw-r--r-- | clang/test/SemaCXX/condition.cpp | 3 | ||||
-rw-r--r-- | clang/test/SemaCXX/if-empty-body.cpp | 35 | ||||
-rw-r--r-- | clang/test/SemaCXX/warn-empty-body.cpp | 271 |
19 files changed, 573 insertions, 77 deletions
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f76dcc2bad7..b042e2f6928 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -4966,8 +4966,20 @@ def err_switch_explicit_conversion : Error< "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">; diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h index 46b97e7113c..2ccb37010cf 100644 --- a/clang/include/clang/Sema/ScopeInfo.h +++ b/clang/include/clang/Sema/ScopeInfo.h @@ -32,6 +32,22 @@ class VarDecl; 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; @@ -79,7 +95,11 @@ public: /// 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. diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 47fa85b9820..45540100628 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -163,6 +163,7 @@ namespace clang { namespace sema { class AccessedEntity; class BlockScopeInfo; + class CompoundScopeInfo; class DelayedDiagnostic; class FunctionScopeInfo; class LambdaScopeInfo; @@ -744,6 +745,11 @@ public: return FunctionScopes.back(); } + void PushCompoundScope(); + void PopCompoundScope(); + + sema::CompoundScopeInfo &getCurCompoundScope() const; + bool hasAnyUnrecoverableErrorsInThisFunction() const; /// \brief Retrieve the current block, if any. @@ -2053,9 +2059,28 @@ public: 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); @@ -2204,6 +2229,21 @@ public: void DiagnoseUnusedExprResult(const Stmt *S); void DiagnoseUnusedDecl(const NamedDecl *ND); + /// Emit \p DiagID if statement located on \p StmtLoc has a suspicious null + /// statement as a \p Body, and it is located on the same line. + /// + /// This helps prevent bugs due to typos, such as: + /// if (condition); + /// do_stuff(); + void DiagnoseEmptyStmtBody(SourceLocation StmtLoc, + const Stmt *Body, + 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(); } diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp index 41dfeddaab9..416cb39a639 100644 --- a/clang/lib/Parse/ParseObjc.cpp +++ b/clang/lib/Parse/ParseObjc.cpp @@ -2635,9 +2635,11 @@ Decl *Parser::ParseLexedObjCMethodDefs(LexedMethod &LM) { 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(); diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp index aa9ba06b67b..8af4da6a846 100644 --- a/clang/lib/Parse/ParseStmt.cpp +++ b/clang/lib/Parse/ParseStmt.cpp @@ -700,7 +700,6 @@ StmtResult Parser::ParseCompoundStatement(ParsedAttributes &attrs, 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 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) { 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,9 +1085,14 @@ StmtResult Parser::ParseSwitchStatement(ParsedAttributes &attrs, 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()->getLocEnd(); + Body = Actions.ActOnNullStmt(SynthesizedNullStmtLocation); + } return Actions.ActOnFinishSwitchStmt(SwitchLoc, Switch.get(), Body.get()); } @@ -1956,9 +1962,11 @@ Decl *Parser::ParseFunctionStatementBody(Decl *Decl, ParseScope &BodyScope) { 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 +2001,11 @@ Decl *Parser::ParseFunctionTryBlock(Decl *Decl, ParseScope &BodyScope) { 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()); diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 42b102f55a0..716ffd131b2 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -861,6 +861,17 @@ void Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP, } } +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 { diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index cf0469592f8..3393cf73f1c 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -4827,3 +4827,130 @@ void Sema::checkUnsafeExprAssigns(SourceLocation Loc, } } } + +//===--- CHECK: Empty statement body (-Wempty-body) ---------------------===// + +namespace { +bool ShouldDiagnoseEmptyStmtBody(const SourceManager &SourceMgr, + SourceLocation StmtLoc, + const NullStmt *Body) { + // 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; + + // 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; +} +} // Unnamed namespace + +void Sema::DiagnoseEmptyStmtBody(SourceLocation StmtLoc, + const Stmt *Body, + unsigned DiagID) { + // Since this is a syntactic check, don't emit diagnostic for template + // instantiations, this just adds noise. + if (CurrentInstantiationScope) + return; + + // The body should be a null statement. + const NullStmt *NBody = dyn_cast<NullStmt>(Body); + if (!NBody) + return; + + // Do the usual checks. + if (!ShouldDiagnoseEmptyStmtBody(SourceMgr, StmtLoc, NBody)) + return; + + Diag(NBody->getSemiLoc(), DiagID); + Diag(NBody->getSemiLoc(), diag::note_empty_body_on_separate_line); +} + +void Sema::DiagnoseEmptyLoopBody(const Stmt *S, + const Stmt *PossibleBody) { + assert(!CurrentInstantiationScope); // Ensured by caller + + 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 diagnostic is disabled. + if (Diags.getDiagnosticLevel(DiagID, NBody->getSemiLoc()) == + DiagnosticsEngine::Ignored) + return; + + // Do the usual checks. + if (!ShouldDiagnoseEmptyStmtBody(SourceMgr, StmtLoc, NBody)) + return; + + // `for(...);' and `while(...);' are popular idioms, so in order to keep + // noise level low, emit diagnostics only if for/while is 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); + } +} + diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 68c938149ea..84063e5fd0f 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -8218,10 +8218,14 @@ void Sema::DefineImplicitCopyAssignment(SourceLocation CurrentLocation, 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()) { @@ -8650,10 +8654,14 @@ void Sema::DefineImplicitMoveAssignment(SourceLocation CurrentLocation, 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()) { @@ -8852,9 +8860,10 @@ void Sema::DefineImplicitCopyConstructor(SourceLocation CurrentLocation, << 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); @@ -9006,9 +9015,10 @@ void Sema::DefineImplicitMoveConstructor(SourceLocation CurrentLocation, << 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); diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 0adfb948009..b47e3d29d17 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -225,6 +225,18 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S) { 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,15 @@ Sema::ActOnCompoundStmt(SourceLocation L, SourceLocation R, DiagnoseUnusedExprResult(Elts[i]); } + // Check for suspicious empty body (null statement) in `for' and `while' + // statements. Don't do anything for template instantiations, this just adds + // noise. + 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)); } @@ -355,21 +376,9 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar, 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(ConditionExpr->getLocEnd(), thenStmt, + diag::warn_empty_if_body); } DiagnoseUnusedExprResult(elseStmt); @@ -957,6 +966,9 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch, } } + DiagnoseEmptyStmtBody(CondExpr->getLocEnd(), BodyStmt, + 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) @@ -983,6 +995,9 @@ Sema::ActOnWhileStmt(SourceLocation WhileLoc, FullExprArg Cond, DiagnoseUnusedExprResult(Body); + if (isa<NullStmt>(Body)) + getCurCompoundScope().setHasEmptyLoopBodies(); + return Owned(new (Context) WhileStmt(Context, ConditionVar, ConditionExpr, Body, WhileLoc)); } @@ -1046,6 +1061,9 @@ Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc, 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,7 +1605,12 @@ StmtResult Sema::FinishCXXForRangeStmt(Stmt *S, Stmt *B) { if (!S || !B) return StmtError(); - cast<CXXForRangeStmt>(S)->setBody(B); + CXXForRangeStmt *ForStmt = cast<CXXForRangeStmt>(S); + ForStmt->setBody(B); + + DiagnoseEmptyStmtBody(ForStmt->getRParenLoc(), B, + diag::warn_empty_range_based_for_body); + return S; } diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 71037b6b3e3..4c69dd51f70 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -5006,6 +5006,8 @@ template<typename Derived> StmtResult TreeTransform<Derived>::TransformCompoundStmt(CompoundStmt *S, bool IsStmtExpr) { + Sema::CompoundScopeRAII CompoundScope(getSema()); + bool SubStmtInvalid = false; bool SubStmtChanged = false; ASTOwningVector<Stmt*> Statements(getSema()); diff --git a/clang/test/Analysis/misc-ps.m b/clang/test/Analysis/misc-ps.m index 0b3ca1be8e2..7c6818a4cdf 100644 --- a/clang/test/Analysis/misc-ps.m +++ b/clang/test/Analysis/misc-ps.m @@ -1248,7 +1248,7 @@ void pr9269() { 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) {} } diff --git a/clang/test/Parser/cxx-condition.cpp b/clang/test/Parser/cxx-condition.cpp index 552d8236246..5b078429299 100644 --- a/clang/test/Parser/cxx-condition.cpp +++ b/clang/test/Parser/cxx-condition.cpp @@ -5,7 +5,7 @@ void f() { while (a) ; while (int x) ; // expected-error {{expected '=' after declarator}} while (float x = 0) ; - if (const int x = a) ; // expected-warning{{empty body}} + if (const int x = a) ; // expected-warning{{empty body}} expected-note{{put the semicolon on a separate line to silence this warning}} switch (int x = a+10) {} for (; int x = ++a; ) ; } diff --git a/clang/test/Parser/statements.c b/clang/test/Parser/statements.c index bc7192a7b2b..3a123d6001b 100644 --- a/clang/test/Parser/statements.c +++ b/clang/test/Parser/statements.c @@ -31,20 +31,20 @@ void test3() { } void test4() { - if (0); // expected-warning {{if statement has empty body}} + if (0); // expected-warning {{if statement has empty body}} expected-note {{put the semicolon on a separate line to silence this warning}} int X; // declaration in a block. -foo: if (0); // expected-warning {{if statement has empty body}} +foo: if (0); // expected-warning {{if statement has empty body}} expected-note {{put the semicolon on a separate line to silence this warning}} } typedef int t; void test5() { - if (0); // expected-warning {{if statement has empty body}} + if (0); // expected-warning {{if statement has empty body}} expected-note {{put the semicolon on a separate line to silence this warning}} t x = 0; - if (0); // expected-warning {{if statement has empty body}} + if (0); // expected-warning {{if statement has empty body}} expected-note {{put the semicolon on a separate line to silence this warning}} } diff --git a/clang/test/Sema/default.c b/clang/test/Sema/default.c index 13186018450..429e63ae2a6 100644 --- a/clang/test/Sema/default.c +++ b/clang/test/Sema/default.c @@ -3,6 +3,6 @@ void f5 (int z) { if (z) default: // expected-error {{not in switch statement}} - ; // expected-warning {{if statement has empty body}} + ; } diff --git a/clang/test/Sema/statements.c b/clang/test/Sema/statements.c index 963b98fe56a..f01ee408671 100644 --- a/clang/test/Sema/statements.c +++ b/clang/test/Sema/statements.c @@ -36,7 +36,7 @@ bar: // PR6034 void test11(int bit) { - switch (bit) + switch (bit) // expected-warning {{switch statement has empty body}} expected-note {{put the semicolon on a separate line to silence this warning}} switch (env->fpscr) // expected-error {{use of undeclared identifier 'env'}} { } diff --git a/clang/test/Sema/switch.c b/clang/test/Sema/switch.c index 8325b4e8237..5c8ebf516c3 100644 --- a/clang/test/Sema/switch.c +++ b/clang/test/Sema/switch.c @@ -24,7 +24,9 @@ void foo(int X) { 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}} \ + // expected-note{{put the semicolon on a separate line to silence this warning}} } extern int g(); diff --git a/clang/test/SemaCXX/condition.cpp b/clang/test/SemaCXX/condition.cpp index 43ee640e0f6..993f8e1d776 100644 --- a/clang/test/SemaCXX/condition.cpp +++ b/clang/test/SemaCXX/condition.cpp @@ -20,7 +20,8 @@ void test() { 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}} \ + // expected-note{{put the semicolon on a separate line}} if (int x=0) { // expected-note 2 {{previous definition is here}} int x; // expected-error {{redefinition of 'x'}} diff --git a/clang/test/SemaCXX/if-empty-body.cpp b/clang/test/SemaCXX/if-empty-body.cpp deleted file mode 100644 index ec7f89d68e7..00000000000 --- a/clang/test/SemaCXX/if-empty-body.cpp +++ /dev/null @@ -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>(); -} diff --git a/clang/test/SemaCXX/warn-empty-body.cpp b/clang/test/SemaCXX/warn-empty-body.cpp new file mode 100644 index 00000000000..d643cedf09c --- /dev/null +++ b/clang/test/SemaCXX/warn-empty-body.cpp @@ -0,0 +1,271 @@ +// 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}} expected-note{{put the semicolon on a separate line to silence this warning}} + + 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}} expected-note{{put the semicolon on a separate line to silence this warning}} + a(i); + + for (int j : + arr); // expected-warning{{range-based for 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); // 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); + } + + do; // expected-note{{to match this 'do'}} + b(); // expected-error{{expected 'while' in do/while loop}} + while (b()); // no-warning + c(); + + do; // expected-note{{to match this 'do'}} + b(); // expected-error{{expected 'while' in do/while loop}} + while (b()); // expected-warning{{while loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}} + c(); + + switch(x) // no-warning + { + switch(y); // expected-warning{{switch statement has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}} + { + case 0: + a(10); + break; + default: + a(20); + break; + } + } + } +} + +/// There should be no warning when null statement is placed on its own line. +void test2(int x, int y) { + if (x) // no-warning + ; // no-warning + + 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) // no-warning + ; // no-warning + + while (b() == 0) // no-warning + ; // no-warning + + while (b() == 0 || + c() == 0) // no-warning + ; // no-warning + + switch(x) + { + switch(y) // no-warning + ; // no-warning + } + + // 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 + } +} + +void test4(int x) +{ + // Idiom used in some metaprogramming constructs. + switch (x) default:; // no-warning + + // Frequent idiom used in macros. + do {} while (false); // 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 test5(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 test6(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 + + while (b() == 0) // no-warning + a(i); // no-warning + + while (b() == 0) {} // 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}} expected-note{{put the semicolon on a separate line to silence this warning}} + + if (x) + EMPTY(x); // no-warning + + int arr[3] = { 1, 2, 3 }; + for (int j : arr); // expected-warning{{range-based for loop has empty body}} expected-note{{put the semicolon on a separate line to silence this 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); +} + |