clang: Tweak behaviour of warn_empty_while_body and warn_empty_if_body

[bmoody]

Summary

Use the if/while statement right paren location instead of the end of the
condition expression to determine if the semicolon is on its own line, for the
purpose of not warning about code like this:

while (foo())
  ;

Using the condition location meant that we would also not report a warning on
code like this:

while (MACRO(a,
             b));
  body();

The right paren loc wasn't stored in the AST or passed into Sema::ActOnIfStmt
when this logic was first written.

[rnk]

lgtm

Test cases look good, thanks!

[bmoody]

I don't have merge access so please merge on my behalf :) Thanks!

clang/lib/Sema/SemaChecking.cpp

void Sema::DiagnoseEmptyLoopBody(const Stmt *S,
   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();
+    StmtLoc = WS->getRParenLoc();
     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)

clang/lib/Sema/SemaStmt.cpp

StmtResult Sema::ActOnIfStmt(SourceLocation IfLoc,
   assert((CondExpr || ConstevalOrNegatedConsteval) &&
          "If statement: missing condition");
   // Only call the CommaVisitor when not C89 due to differences in scope flags.
   if (CondExpr && (getLangOpts().C99 || getLangOpts().CPlusPlus) &&
       !Diags.isIgnored(diag::warn_comma_operator, CondExpr->getExprLoc()))
     CommaVisitor(*this).Visit(CondExpr);
 
   if (!ConstevalOrNegatedConsteval && !elseStmt)
-    DiagnoseEmptyStmtBody(CondExpr->getEndLoc(), thenStmt,
-                          diag::warn_empty_if_body);
+    DiagnoseEmptyStmtBody(RParenLoc, thenStmt, diag::warn_empty_if_body);
 
   if (ConstevalOrNegatedConsteval ||
       StatementKind == IfStatementKind::Constexpr) {
     auto DiagnoseLikelihood = [&](const Stmt *S) {
       if (const Attr *A = Stmt::getLikelihoodAttr(S)) {
         Diags.Report(A->getLocation(),
                      diag::warn_attribute_has_no_effect_on_compile_time_if)
             << A << ConstevalOrNegatedConsteval << A->getRange();

clang/test/CXX/stmt.stmt/stmt.select/p3.cpp

void whileInitStatement() {
   // expected-note@-1 {{to match this '('}}
   // expected-error@-2 {{use of undeclared identifier 'I'}}
 
   int Var = 10;
   while (Var + Var; Var--) {} // expected-error {{expected ')'}}
   // expected-note@-1 {{to match this '('}}
   // expected-error@-2 {{expected ';' after expression}}
   // expected-error@-3 {{expected expression}}
-  // expected-warning@-4 {{while loop has empty body}}
-  // expected-note@-5 {{put the semicolon on a separate line to silence this warning}}
 }
 
 // TODO: This is needed because clang can't seem to diagnose invalid syntax after the
 // last loop above. It would be nice to remove this.
 void whileInitStatement2() {
   while (; false) {} // expected-error {{expected expression}}
   // expected-error@-1 {{expected ';' after expression}}
   // expected-error@-2 {{expected expression}}
 }

clang/test/SemaCXX/warn-empty-body.cpp

 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
 
 void a(int i);
 int b();
 int c();
 
 #define MACRO_A 0
 
+#define AND(x, y) ((x) && (y))
+
 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}}
 
     // Check that we handle conditions that start or end with a macro
     // correctly.
     if (x == MACRO_A); // expected-warning {{if statement has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
     if (MACRO_A == x); // expected-warning {{if statement has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
 
+    // Check that we handle the case where the condition comes from a macro
+    // expansion over multiple lines.
+    if (AND(b(),
+            c())); // expected-warning {{if statement has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+
+    while (AND(b(),
+               c())); // expected-warning{{while loop has empty body}} expected-note{{put the semicolon on a separate line to silence this warning}}
+      a(0);
+
     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}}