bmoody created this revision.
bmoody added reviewers: rnk, gribozavr.
Herald added a project: All.
bmoody requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128406

Files:
  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


Index: clang/test/SemaCXX/warn-empty-body.cpp
===================================================================
--- clang/test/SemaCXX/warn-empty-body.cpp
+++ clang/test/SemaCXX/warn-empty-body.cpp
@@ -6,6 +6,8 @@
 
 #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 @@
     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}}
Index: clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
===================================================================
--- clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -63,8 +63,6 @@
   // 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
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -888,8 +888,7 @@
     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) {
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -16673,7 +16673,7 @@
     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


Index: clang/test/SemaCXX/warn-empty-body.cpp
===================================================================
--- clang/test/SemaCXX/warn-empty-body.cpp
+++ clang/test/SemaCXX/warn-empty-body.cpp
@@ -6,6 +6,8 @@
 
 #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 @@
     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}}
Index: clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
===================================================================
--- clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
+++ clang/test/CXX/stmt.stmt/stmt.select/p3.cpp
@@ -63,8 +63,6 @@
   // 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
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -888,8 +888,7 @@
     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) {
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -16673,7 +16673,7 @@
     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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to