Skip to content

[sema] enhance error handling for compound stmt body in StmtExpr #113760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

HerrCai0907
Copy link
Contributor

@HerrCai0907 HerrCai0907 commented Oct 26, 2024

Mark the whole StmtExpr invalid when the last statement in compound statement is invalid.
Because the last statement need to do copy initialization, it causes subsequent errors to simply ignore last invalid statement.

Fixed: #113468

Mark the whole StmtExpr invalid when the last statement in compound statement is invalid.
Because the last statement need to do copy initialization, it causes subsequent errors to simply ignore last invalid statement.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 26, 2024

@llvm/pr-subscribers-clang

Author: Congcong Cai (HerrCai0907)

Changes

Mark the whole StmtExpr invalid when the last statement in compound statement is invalid.
Because the last statement need to do copy initialization, it causes subsequent errors to simply ignore last invalid statement.


Full diff: https://github.com/llvm/llvm-project/pull/113760.diff

2 Files Affected:

  • (modified) clang/lib/Parse/ParseStmt.cpp (+9)
  • (added) clang/test/SemaCXX/gh113468.cpp (+12)
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 60d647da48f053..6d0de38095b999 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1243,6 +1243,7 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
       ParsedStmtContext::Compound |
       (isStmtExpr ? ParsedStmtContext::InStmtExpr : ParsedStmtContext());
 
+  bool LastIsError = false;
   while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
          Tok.isNot(tok::eof)) {
     if (Tok.is(tok::annot_pragma_unused)) {
@@ -1299,7 +1300,15 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
 
     if (R.isUsable())
       Stmts.push_back(R.get());
+    LastIsError = R.isInvalid();
   }
+  // StmtExpr needs to do copy initialization for last statement.
+  // If last statement is invalid, the last statement in `Stmts` will be
+  // incorrect. Then the whole compound statement should also be marked as
+  // invalid to prevent subsequent errors.
+  if (isStmtExpr && LastIsError && !Stmts.empty())
+    return StmtError();
+
   // Warn the user that using option `-ffp-eval-method=source` on a
   // 32-bit target and feature `sse` disabled, or using
   // `pragma clang fp eval_method=source` and feature `sse` disabled, is not
diff --git a/clang/test/SemaCXX/gh113468.cpp b/clang/test/SemaCXX/gh113468.cpp
new file mode 100644
index 00000000000000..94551986b0efaa
--- /dev/null
+++ b/clang/test/SemaCXX/gh113468.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+constexpr int expr() {
+  if (({
+        int f;
+        f = 0;
+        if (f)
+          break; // expected-error {{'break' statement not in loop or switch statement}}
+      }))
+    return 2;
+  return 1;
+}

Copy link
Member

@Sirraide Sirraide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix lgtm, but this is still missing a release note.

@HerrCai0907 HerrCai0907 merged commit cad0940 into llvm:main Oct 30, 2024
4 of 6 checks passed
@HerrCai0907 HerrCai0907 deleted the 113468-clang-assertion-e-getlhs-gettype-isintegralorenumerationtype- branch October 31, 2024 15:55
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
…lvm#113760)

Mark the whole StmtExpr invalid when the last statement in compound
statement is invalid.
Because the last statement need to do copy initialization, it causes
subsequent errors to simply ignore last invalid statement.

Fixed: llvm#113468
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
3 participants