[clang] [Clang] enhance error recovery with RecoveryExpr for trailing commas in call arguments (PR #114684)
Oleksandr T. via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 20 01:47:44 PST 2024
https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/114684
>From d95d0fdb22ae2ad162f89cb211f313cea6c6474a Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Sat, 2 Nov 2024 23:54:35 +0200
Subject: [PATCH 1/2] [Clang] enhance error recovery with RecoveryExpr for
trailing commas in call arguments
---
clang/lib/Parse/ParseExpr.cpp | 3 +++
clang/test/AST/ast-dump-recovery.cpp | 17 ++++++++++++++++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 4570a18bc0d5e5..5fccd2ae106015 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -3705,6 +3705,9 @@ bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
Token Comma = Tok;
ConsumeToken();
checkPotentialAngleBracketDelimiter(Comma);
+
+ if (Tok.is(tok::r_paren))
+ break;
}
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 a88dff471d9f04..1876f4ace32a5a 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -9,7 +9,7 @@ int some_func(int *);
// CHECK-NEXT: `-IntegerLiteral {{.*}} 123
// DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
int invalid_call = some_func(123);
-void test_invalid_call(int s) {
+void test_invalid_call_1(int s) {
// CHECK: CallExpr {{.*}} '<dependent type>' contains-errors
// CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
// CHECK-NEXT: |-RecoveryExpr {{.*}} <col:13>
@@ -32,6 +32,21 @@ void test_invalid_call(int s) {
int var = some_func(undef1);
}
+int some_func2(int a, int b);
+void test_invalid_call_2() {
+ // CHECK: `-RecoveryExpr {{.*}} 'int' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+ some_func2(1, );
+}
+
+void test_invalid_call_3() {
+ // CHECK: `-RecoveryExpr {{.*}} 'int' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+ some_func2(1);
+}
+
int ambig_func(double);
int ambig_func(float);
>From 2b45d342c7e2327c525b681a0e4069132dcd430d Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Wed, 20 Nov 2024 11:47:13 +0200
Subject: [PATCH 2/2] explicity handle of trailing commas to improve recovery
and ensure proper processing of prior erroneous expressions
---
clang/include/clang/Parse/Parser.h | 3 ++-
clang/lib/Parse/ParseDecl.cpp | 5 +++--
clang/lib/Parse/ParseExpr.cpp | 19 +++++++++++++++----
clang/test/AST/ast-dump-recovery.cpp | 11 ++++++++---
4 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 045ee754a242b3..d3838a4cc8418c 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1934,7 +1934,8 @@ class Parser : public CodeCompletionHandler {
llvm::function_ref<void()> ExpressionStarts =
llvm::function_ref<void()>(),
bool FailImmediatelyOnInvalidExpr = false,
- bool EarlyTypoCorrection = false);
+ bool EarlyTypoCorrection = false,
+ bool *HasTrailingComma = nullptr);
/// ParseSimpleExpressionList - A simple comma-separated list of expressions,
/// used for misc language extensions.
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index aa5c2d28d429ac..ae8611207b2609 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2890,8 +2890,9 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
// ProduceConstructorSignatureHelp only on VarDecls.
ExpressionStarts = SetPreferredType;
}
-
- bool SawError = ParseExpressionList(Exprs, ExpressionStarts);
+ bool HasTrailingComma = false;
+ bool SawError =
+ ParseExpressionList(Exprs, ExpressionStarts, HasTrailingComma);
if (SawError) {
if (ThisVarDecl && PP.isCodeCompletionReached() && !CalledSignatureHelp) {
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 5fccd2ae106015..736484ded8383c 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -2199,10 +2199,17 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
};
if (OpKind == tok::l_paren || !LHS.isInvalid()) {
if (Tok.isNot(tok::r_paren)) {
- if (ParseExpressionList(ArgExprs, [&] {
+ bool HasTrailingComma = false;
+ bool HasError = ParseExpressionList(
+ ArgExprs,
+ [&] {
PreferredType.enterFunctionArgument(Tok.getLocation(),
RunSignatureHelp);
- })) {
+ },
+ /*FailImmediatelyOnInvalidExpr*/ false,
+ /*EarlyTypoCorrection*/ false, &HasTrailingComma);
+
+ if (HasError && !HasTrailingComma) {
(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
@@ -3662,7 +3669,8 @@ void Parser::injectEmbedTokens() {
bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
llvm::function_ref<void()> ExpressionStarts,
bool FailImmediatelyOnInvalidExpr,
- bool EarlyTypoCorrection) {
+ bool EarlyTypoCorrection,
+ bool *HasTrailingComma) {
bool SawError = false;
while (true) {
if (ExpressionStarts)
@@ -3706,8 +3714,11 @@ bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
ConsumeToken();
checkPotentialAngleBracketDelimiter(Comma);
- if (Tok.is(tok::r_paren))
+ if (Tok.is(tok::r_paren)) {
+ if (HasTrailingComma)
+ *HasTrailingComma = true;
break;
+ }
}
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 1876f4ace32a5a..37ce02b5e07982 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -42,9 +42,14 @@ void test_invalid_call_2() {
void test_invalid_call_3() {
// CHECK: `-RecoveryExpr {{.*}} 'int' contains-errors
- // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
- // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
- some_func2(1);
+ // CHECK-NEXT: -UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
+ some_func2(,);
+}
+
+void test_invalid_call_4() {
+ // CHECK: `-RecoveryExpr {{.*}} 'int' contains-errors
+ // CHECK-NEXT: -UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
+ some_func2(,,);
}
int ambig_func(double);
More information about the cfe-commits
mailing list