[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
Thu Nov 21 03:20:25 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/6] [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/6] 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);
>From 7b9a6ae366276352bb8ac5bab684f6060bd544c7 Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Wed, 20 Nov 2024 14:05:11 +0200
Subject: [PATCH 3/6] cleanup
---
clang/lib/Parse/ParseDecl.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index ae8611207b2609..ecc7a9f20aafe8 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2890,9 +2890,7 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
// ProduceConstructorSignatureHelp only on VarDecls.
ExpressionStarts = SetPreferredType;
}
- bool HasTrailingComma = false;
- bool SawError =
- ParseExpressionList(Exprs, ExpressionStarts, HasTrailingComma);
+ bool SawError = ParseExpressionList(Exprs, ExpressionStarts);
if (SawError) {
if (ThisVarDecl && PP.isCodeCompletionReached() && !CalledSignatureHelp) {
>From e1f6e8dfd1bdfeea11350d8818ba4ad308cac5cb Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Wed, 20 Nov 2024 14:05:45 +0200
Subject: [PATCH 4/6] update release notes
---
clang/docs/ReleaseNotes.rst | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d496fe7eeb0d8f..855aa6774f0885 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -558,6 +558,8 @@ Improvements to Clang's diagnostics
- Clang now diagnoses ``= delete("reason")`` extension warnings only in pedantic mode rather than on by default. (#GH109311).
+- Improved error recovery for function call arguments with trailing commas (#GH100921).
+
Improvements to Clang's time-trace
----------------------------------
>From 802a273450baa882ade95c5b225719bf62581f24 Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Wed, 20 Nov 2024 14:07:02 +0200
Subject: [PATCH 5/6] revert removed line
---
clang/lib/Parse/ParseDecl.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index ecc7a9f20aafe8..aa5c2d28d429ac 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2890,6 +2890,7 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
// ProduceConstructorSignatureHelp only on VarDecls.
ExpressionStarts = SetPreferredType;
}
+
bool SawError = ParseExpressionList(Exprs, ExpressionStarts);
if (SawError) {
>From 28bb6d43a98691b11cfc439f1171217a96d5c28e Mon Sep 17 00:00:00 2001
From: Oleksandr T <oleksandr.tarasiuk at outlook.com>
Date: Thu, 21 Nov 2024 02:14:57 +0200
Subject: [PATCH 6/6] update tests
---
clang/test/AST/ast-dump-recovery.cpp | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp
index 37ce02b5e07982..02eb6067e82a6d 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -34,22 +34,21 @@ 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-NEXT: `-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
+ some_func2(,);
+
+ // CHECK: -RecoveryExpr {{.*}} 'int' contains-errors
+ // CHECK-NEXT: `-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
+ some_func2(,,);
+
// CHECK: `-RecoveryExpr {{.*}} 'int' contains-errors
// CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
- some_func2(1, );
-}
+ some_func2(1,);
-void test_invalid_call_3() {
- // CHECK: `-RecoveryExpr {{.*}} 'int' contains-errors
- // 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(,,);
+ // CHECK-NOT: `-RecoveryExpr
+ some_func2(,1);
}
int ambig_func(double);
More information about the cfe-commits
mailing list