Skip to content

Commit b144258

Browse files
zyn0217zyn0217
authored
[Clang] Improve error recovery for invalid calls (llvm#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)
1 parent 46e7347 commit b144258

File tree

6 files changed

+31
-29
lines changed

6 files changed

+31
-29
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,14 +398,16 @@ Improvements to Clang's diagnostics
398398
constructors to initialize their non-modifiable members. The diagnostic is
399399
not new; being controlled via a warning group is what's new. Fixes #GH41104
400400

401+
402+
- Improved Clang's error recovery for invalid function calls.
403+
401404
- Improved bit-field diagnostics to consider the type specified by the
402405
``preferred_type`` attribute. These diagnostics are controlled by the flags
403406
``-Wpreferred-type-bitfield-enum-conversion`` and
404407
``-Wpreferred-type-bitfield-width``. These warnings are on by default as they
405408
they're only triggered if the authors are already making the choice to use
406409
``preferred_type`` attribute.
407410

408-
409411
Improvements to Clang's time-trace
410412
----------------------------------
411413

‎clang/include/clang/Parse/Parser.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1942,8 +1942,7 @@ class Parser : public CodeCompletionHandler {
19421942
llvm::function_ref<void()> ExpressionStarts =
19431943
llvm::function_ref<void()>(),
19441944
bool FailImmediatelyOnInvalidExpr = false,
1945-
bool EarlyTypoCorrection = false,
1946-
bool *HasTrailingComma = nullptr);
1945+
bool EarlyTypoCorrection = false);
19471946

19481947
/// ParseSimpleExpressionList - A simple comma-separated list of expressions,
19491948
/// used for misc language extensions.

‎clang/lib/Parse/ParseExpr.cpp

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2218,27 +2218,20 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
22182218
CalledSignatureHelp = true;
22192219
return PreferredType;
22202220
};
2221+
bool ExpressionListIsInvalid = false;
22212222
if (OpKind == tok::l_paren || !LHS.isInvalid()) {
22222223
if (Tok.isNot(tok::r_paren)) {
2223-
bool HasTrailingComma = false;
2224-
bool HasError = ParseExpressionList(
2225-
ArgExprs,
2226-
[&] {
2227-
PreferredType.enterFunctionArgument(Tok.getLocation(),
2228-
RunSignatureHelp);
2229-
},
2230-
/*FailImmediatelyOnInvalidExpr*/ false,
2231-
/*EarlyTypoCorrection*/ false, &HasTrailingComma);
2232-
2233-
if (HasError && !HasTrailingComma) {
2224+
if ((ExpressionListIsInvalid = ParseExpressionList(ArgExprs, [&] {
2225+
PreferredType.enterFunctionArgument(Tok.getLocation(),
2226+
RunSignatureHelp);
2227+
}))) {
22342228
(void)Actions.CorrectDelayedTyposInExpr(LHS);
22352229
// If we got an error when parsing expression list, we don't call
22362230
// the CodeCompleteCall handler inside the parser. So call it here
22372231
// to make sure we get overload suggestions even when we are in the
22382232
// middle of a parameter.
22392233
if (PP.isCodeCompletionReached() && !CalledSignatureHelp)
22402234
RunSignatureHelp();
2241-
LHS = ExprError();
22422235
} else if (LHS.isInvalid()) {
22432236
for (auto &E : ArgExprs)
22442237
Actions.CorrectDelayedTyposInExpr(E);
@@ -2249,6 +2242,12 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
22492242
// Match the ')'.
22502243
if (LHS.isInvalid()) {
22512244
SkipUntil(tok::r_paren, StopAtSemi);
2245+
} else if (ExpressionListIsInvalid) {
2246+
Expr *Fn = LHS.get();
2247+
ArgExprs.insert(ArgExprs.begin(), Fn);
2248+
LHS = Actions.CreateRecoveryExpr(Fn->getBeginLoc(), Tok.getLocation(),
2249+
ArgExprs);
2250+
SkipUntil(tok::r_paren, StopAtSemi);
22522251
} else if (Tok.isNot(tok::r_paren)) {
22532252
bool HadDelayedTypo = false;
22542253
if (Actions.CorrectDelayedTyposInExpr(LHS).get() != LHS.get())
@@ -3700,8 +3699,7 @@ void Parser::injectEmbedTokens() {
37003699
bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
37013700
llvm::function_ref<void()> ExpressionStarts,
37023701
bool FailImmediatelyOnInvalidExpr,
3703-
bool EarlyTypoCorrection,
3704-
bool *HasTrailingComma) {
3702+
bool EarlyTypoCorrection) {
37053703
bool SawError = false;
37063704
while (true) {
37073705
if (ExpressionStarts)
@@ -3744,11 +3742,6 @@ bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
37443742
Token Comma = Tok;
37453743
ConsumeToken();
37463744
checkPotentialAngleBracketDelimiter(Comma);
3747-
3748-
if (Tok.is(tok::r_paren)) {
3749-
if (HasTrailingComma)
3750-
*HasTrailingComma = true;
3751-
}
37523745
}
37533746
if (SawError) {
37543747
// Ensure typos get diagnosed when errors were encountered while parsing the

‎clang/test/AST/ast-dump-recovery.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,21 +34,22 @@ void test_invalid_call_1(int s) {
3434

3535
int some_func2(int a, int b);
3636
void test_invalid_call_2() {
37-
// CHECK: -RecoveryExpr {{.*}} 'int' contains-errors
37+
// CHECK: -RecoveryExpr {{.*}} '<dependent type>' contains-errors
3838
// CHECK-NEXT: `-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
3939
some_func2(,);
4040

41-
// CHECK: -RecoveryExpr {{.*}} 'int' contains-errors
41+
// CHECK: -RecoveryExpr {{.*}} '<dependent type>' contains-errors
4242
// CHECK-NEXT: `-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
4343
some_func2(,,);
4444

45-
// CHECK: `-RecoveryExpr {{.*}} 'int' contains-errors
45+
// CHECK: -RecoveryExpr {{.*}} '<dependent type>' contains-errors
4646
// CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
4747
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
4848
some_func2(1,);
4949

50-
// FIXME: Handle invalid argument with recovery
51-
// CHECK-NOT: `-RecoveryExpr
50+
// CHECK: -RecoveryExpr {{.*}} '<dependent type>' contains-errors
51+
// CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'some_func2'
52+
// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
5253
some_func2(,1);
5354
}
5455

‎clang/test/AST/new-unknown-type.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,15 @@ namespace b {
1717
// CHECK: |-NamespaceDecl 0x{{[^ ]*}} <line:5:1, line:11:1> line:5:11 a
1818
// CHECK-NEXT: | `-FunctionDecl 0x{{[^ ]*}} <line:6:3, line:10:3> line:6:8 computeSomething 'void ()'
1919
// CHECK-NEXT: | `-CompoundStmt 0x{{[^ ]*}} <col:27, line:10:3>
20+
// CHECK-NEXT: | |-RecoveryExpr {{.*}} '<dependent type>' contains-errors
21+
// CHECK-NEXT: | | `-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'foo'
22+
// CHECK-NEXT: | |-RecoveryExpr {{.*}} '<dependent type>' contains-errors
23+
// CHECK-NEXT: | | `-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'foo'
24+
// CHECK-NEXT: | `-RecoveryExpr {{.*}} '<dependent type>' contains-errors
25+
// CHECK-NEXT: | `-UnresolvedLookupExpr {{.*}} '<overloaded function type>' lvalue (ADL) = 'foo'
2026
// CHECK-NEXT: |-NamespaceDecl 0x{{[^ ]*}} <line:13:1, line:15:1> line:13:11 b
2127
// CHECK-NEXT: | `-CXXRecordDecl 0x{{[^ ]*}} <line:14:3, col:14> col:10 referenced struct Bar definition
2228

2329
static b::Bar bar;
24-
// CHECK: `-VarDecl 0x{{[^ ]*}} <line:23:1, col:15> col:15 bar 'b::Bar' static callinit
30+
// CHECK: `-VarDecl 0x{{[^ ]*}} <line:29:1, col:15> col:15 bar 'b::Bar' static callinit
2531
// CHECK-NEXT: `-CXXConstructExpr 0x{{[^ ]*}} <col:15> 'b::Bar' 'void () noexcept'

‎clang/test/Parser/cxx-concepts-requires-clause.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,8 @@ template<typename T> requires 0
119119

120120
template<typename T> requires foo<T>
121121
(int) bar() { };
122-
// expected-error@-1{{expected '(' for function-style cast or type construction}}
122+
// expected-error@-1{{expected '(' for function-style cast or type construction}} \
123+
// expected-error@-2{{parentheses are required around this expression in a requires clause}}
123124

124125
template<typename T>
125126
void bar() requires foo<T>();

0 commit comments

Comments
 (0)