[Clang] enhance error recovery with RecoveryExpr for trailing commas in call arguments

clang/docs/ReleaseNotes.rst

@@ -578,6 +578,8 @@ Improvements to Clang's diagnostics
 
 - Clang now omits shadowing warnings for parameter names in explicit object member functions (#GH95707).
 
+- Improved error recovery for function call arguments with trailing commas (#GH100921).
+
 Improvements to Clang's time-trace
 ----------------------------------
 

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.

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)
@@ -3705,6 +3713,12 @@ bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
     Token Comma = Tok;
     ConsumeToken();
     checkPotentialAngleBracketDelimiter(Comma);
+
+    if (Tok.is(tok::r_paren)) {
+      if (HasTrailingComma)
+        *HasTrailingComma = true;
+      break;
+    }
   }
   if (SawError) {
     // Ensure typos get diagnosed when errors were encountered while parsing the

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,26 @@ 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'
+  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,);
+
+  // FIXME: Handle invalid argument with recovery
+  // CHECK-NOT: `-RecoveryExpr
+  some_func2(,1);
+}
+
 int ambig_func(double);
 int ambig_func(float);