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.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@150515 91177308-0d34-0410-b5e6-96231b3b80d8

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">;

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.

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();
   }

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();

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());

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 {

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);
+  }
+}
+

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);

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;
 }
 

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());

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) {}
 }
 

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; ) ;
 }

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}}
 }
 
 

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}}
+      ;
 } 
 

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'}}
   {
   }

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();

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'}}

test/SemaCXX/if-empty-body.cpp

@@ -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>();
-}

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);
+}
+