[clang] 37b881a - clang: Tweak behaviour of warn_empty_while_body and warn_empty_if_body

[Dmitri Gribenko via cfe-commits]

Author: Dmitri Gribenko
Date: 2022-06-24T02:40:25+02:00
New Revision: 37b881aa0bca09b7cc80575cc4b97453aa724186

URL: https://github.com/llvm/llvm-project/commit/37b881aa0bca09b7cc80575cc4b97453aa724186
DIFF: https://github.com/llvm/llvm-project/commit/37b881aa0bca09b7cc80575cc4b97453aa724186.diff

LOG: clang: Tweak behaviour of warn_empty_while_body and warn_empty_if_body

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.

Reviewed By: rnk, gribozavr2

Differential Revision: https://reviews.llvm.org/D128406

Added: 
    

Modified: 
    clang/lib/Sema/SemaChecking.cpp
    clang/lib/Sema/SemaStmt.cpp
    clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
    clang/test/SemaCXX/warn-empty-body.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 58f803ed00243..4357e45c6a14a 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -16673,7 +16673,7 @@ void Sema::DiagnoseEmptyLoopBody(const Stmt *S,
     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

diff  --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index ae3ea4db233e5..82831a448869d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -888,8 +888,7 @@ StmtResult Sema::ActOnIfStmt(SourceLocation IfLoc,
     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) {

diff  --git a/clang/test/CXX/stmt.stmt/stmt.select/p3.cpp b/clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
index 4804cc559d007..2d45d82d43754 100644
--- a/clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ b/clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -63,8 +63,6 @@ void whileInitStatement() {
   // expected-note at -1 {{to match this '('}}
   // expected-error at -2 {{expected ';' after expression}}
   // expected-error at -3 {{expected expression}}
-  // expected-warning at -4 {{while loop has empty body}}
-  // expected-note at -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

diff  --git a/clang/test/SemaCXX/warn-empty-body.cpp b/clang/test/SemaCXX/warn-empty-body.cpp
index a248c4251d525..08cfb57e90e07 100644
--- a/clang/test/SemaCXX/warn-empty-body.cpp
+++ b/clang/test/SemaCXX/warn-empty-body.cpp
@@ -6,6 +6,8 @@ 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}}
@@ -15,6 +17,15 @@ void test1(int x, int y) {
     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}}