[clang] b144258 - [Clang] Improve error recovery for invalid calls (#136295)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 21 19:41:19 PDT 2025
Author: Younan Zhang
Date: 2025-04-22T10:41:16+08:00
New Revision: b144258b0c0cc63dffba00a911d6539f00ed07bb
URL: https://github.com/llvm/llvm-project/commit/b144258b0c0cc63dffba00a911d6539f00ed07bb
DIFF: https://github.com/llvm/llvm-project/commit/b144258b0c0cc63dffba00a911d6539f00ed07bb.diff
LOG: [Clang] Improve error recovery for invalid calls (#136295)
It doesn't make sense that we only build a RecoveryExpr for expressions
with invalid trailing commas. This patch extends it so that we now
always build up a RecoveryExpr whenever the call contains anything
invalid. As a result, we can back out HasTrailingComma.
There is only one diagnostic change as to concepts, where a RecoveryExpr
than an ExprError is now used to model an invalid requires clause, for
which we suggest adding parentheses around it. (This looks like what GCC
diagnoses)
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Parse/Parser.h
clang/lib/Parse/ParseExpr.cpp
clang/test/AST/ast-dump-recovery.cpp
clang/test/AST/new-unknown-type.cpp
clang/test/Parser/cxx-concepts-requires-clause.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 36b528d9e20f8..86d37f5616356 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -398,6 +398,9 @@ Improvements to Clang's diagnostics
constructors to initialize their non-modifiable members. The diagnostic is
not new; being controlled via a warning group is what's new. Fixes #GH41104
+
+- Improved Clang's error recovery for invalid function calls.
+
- Improved bit-field diagnostics to consider the type specified by the
``preferred_type`` attribute. These diagnostics are controlled by the flags
``-Wpreferred-type-bitfield-enum-conversion`` and
@@ -405,7 +408,6 @@ Improvements to Clang's diagnostics
they're only triggered if the authors are already making the choice to use
``preferred_type`` attribute.
-
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 7ffe23c73eda9..645df4aabc374 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1942,8 +1942,7 @@ class Parser : public CodeCompletionHandler {
llvm::function_ref<void()> ExpressionStarts =
llvm::function_ref<void()>(),
bool FailImmediatelyOnInvalidExpr = false,
- bool EarlyTypoCorrection = false,
- bool *HasTrailingComma = nullptr);
+ bool EarlyTypoCorrection = false);
/// ParseSimpleExpressionList - A simple comma-separated list of expressions,
/// used for misc language extensions.
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 3e3e7cbcd68b2..59ce8a998969d 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -2218,19 +2218,13 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
CalledSignatureHelp = true;
return PreferredType;
};
+ bool ExpressionListIsInvalid = false;
if (OpKind == tok::l_paren || !LHS.isInvalid()) {
if (Tok.isNot(tok::r_paren)) {
- bool HasTrailingComma = false;
- bool HasError = ParseExpressionList(
- ArgExprs,
- [&] {
- PreferredType.enterFunctionArgument(Tok.getLocation(),
- RunSignatureHelp);
- },
- /*FailImmediatelyOnInvalidExpr*/ false,
- /*EarlyTypoCorrection*/ false, &HasTrailingComma);
-
- if (HasError && !HasTrailingComma) {
+ if ((ExpressionListIsInvalid = ParseExpressionList(ArgExprs, [&] {
+ PreferredType.enterFunctionArgument(Tok.getLocation(),
+ RunSignatureHelp);
+ }))) {
(void)Actions.CorrectDelayedTyposInExpr(LHS);
// If we got an error when parsing expression list, we don't call
// the CodeCompleteCall handler inside the parser. So call it here
@@ -2238,7 +2232,6 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
// middle of a parameter.
if (PP.isCodeCompletionReached() && !CalledSignatureHelp)
RunSignatureHelp();
- LHS = ExprError();
} else if (LHS.isInvalid()) {
for (auto &E : ArgExprs)
Actions.CorrectDelayedTyposInExpr(E);
@@ -2249,6 +2242,12 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
// Match the ')'.
if (LHS.isInvalid()) {
SkipUntil(tok::r_paren, StopAtSemi);
+ } else if (ExpressionListIsInvalid) {
+ Expr *Fn = LHS.get();
+ ArgExprs.insert(ArgExprs.begin(), Fn);
+ LHS = Actions.CreateRecoveryExpr(Fn->getBeginLoc(), Tok.getLocation(),
+ ArgExprs);
+ SkipUntil(tok::r_paren, StopAtSemi);
} else if (Tok.isNot(tok::r_paren)) {
bool HadDelayedTypo = false;
if (Actions.CorrectDelayedTyposInExpr(LHS).get() != LHS.get())
@@ -3700,8 +3699,7 @@ void Parser::injectEmbedTokens() {
bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
llvm::function_ref<void()> ExpressionStarts,
bool FailImmediatelyOnInvalidExpr,
- bool EarlyTypoCorrection,
- bool *HasTrailingComma) {
+ bool EarlyTypoCorrection) {
bool SawError = false;
while (true) {
if (ExpressionStarts)
@@ -3744,11 +3742,6 @@ bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
Token Comma = Tok;
ConsumeToken();
checkPotentialAngleBracketDelimiter(Comma);
-
- if (Tok.is(tok::r_paren)) {
- if (HasTrailingComma)
- *HasTrailingComma = true;
- }
}
if (SawError) {
// Ensure typos get diagnosed when errors were encountered while parsing the
diff --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp
index b59fa3778192f..b8195950f2fa1 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -34,21 +34,22 @@ void test_invalid_call_1(int s) {
int some_func2(int a, int b);
void test_invalid_call_2() {
- // CHECK: -RecoveryExpr {{.*}} 'int' contains-errors
+ // CHECK: -RecoveryExpr {{.*}} '<dependent type>' contains-errors
// CHECK-NEXT: `-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
some_func2(,);
- // CHECK: -RecoveryExpr {{.*}} 'int' contains-errors
+ // CHECK: -RecoveryExpr {{.*}} '<dependent type>' contains-errors
// CHECK-NEXT: `-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
some_func2(,,);
- // CHECK: `-RecoveryExpr {{.*}} 'int' contains-errors
+ // CHECK: -RecoveryExpr {{.*}} '<dependent type>' contains-errors
// CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
some_func2(1,);
- // FIXME: Handle invalid argument with recovery
- // CHECK-NOT: `-RecoveryExpr
+ // CHECK: -RecoveryExpr {{.*}} '<dependent type>' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
some_func2(,1);
}
diff --git a/clang/test/AST/new-unknown-type.cpp b/clang/test/AST/new-unknown-type.cpp
index e7dd3c90145d2..9d6b19acba739 100644
--- a/clang/test/AST/new-unknown-type.cpp
+++ b/clang/test/AST/new-unknown-type.cpp
@@ -17,9 +17,15 @@ namespace b {
// CHECK: |-NamespaceDecl 0x{{[^ ]*}} <line:5:1, line:11:1> line:5:11 a
// CHECK-NEXT: | `-FunctionDecl 0x{{[^ ]*}} <line:6:3, line:10:3> line:6:8 computeSomething 'void ()'
// CHECK-NEXT: | `-CompoundStmt 0x{{[^ ]*}} <col:27, line:10:3>
+// CHECK-NEXT: | |-RecoveryExpr {{.*}} '<dependent type>' contains-errors
+// CHECK-NEXT: | | `-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'foo'
+// CHECK-NEXT: | |-RecoveryExpr {{.*}} '<dependent type>' contains-errors
+// CHECK-NEXT: | | `-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'foo'
+// CHECK-NEXT: | `-RecoveryExpr {{.*}} '<dependent type>' contains-errors
+// CHECK-NEXT: | `-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'foo'
// CHECK-NEXT: |-NamespaceDecl 0x{{[^ ]*}} <line:13:1, line:15:1> line:13:11 b
// CHECK-NEXT: | `-CXXRecordDecl 0x{{[^ ]*}} <line:14:3, col:14> col:10 referenced struct Bar definition
static b::Bar bar;
-// CHECK: `-VarDecl 0x{{[^ ]*}} <line:23:1, col:15> col:15 bar 'b::Bar' static callinit
+// CHECK: `-VarDecl 0x{{[^ ]*}} <line:29:1, col:15> col:15 bar 'b::Bar' static callinit
// CHECK-NEXT: `-CXXConstructExpr 0x{{[^ ]*}} <col:15> 'b::Bar' 'void () noexcept'
diff --git a/clang/test/Parser/cxx-concepts-requires-clause.cpp b/clang/test/Parser/cxx-concepts-requires-clause.cpp
index 13b4e8439882d..b921be5dbfd6d 100644
--- a/clang/test/Parser/cxx-concepts-requires-clause.cpp
+++ b/clang/test/Parser/cxx-concepts-requires-clause.cpp
@@ -119,7 +119,8 @@ template<typename T> requires 0
template<typename T> requires foo<T>
(int) bar() { };
-// expected-error at -1{{expected '(' for function-style cast or type construction}}
+// expected-error at -1{{expected '(' for function-style cast or type construction}} \
+// expected-error at -2{{parentheses are required around this expression in a requires clause}}
template<typename T>
void bar() requires foo<T>();
More information about the cfe-commits
mailing list