[AST][RecoveryExpr] Preserve the invalid "undef_var" initializer.

[hokein]

Summary

And don't invalidate the VarDecl if the type is known.

[hokein]

clang/include/clang/Sema/Sema.h

3900 ↗

(On Diff #269218)

not quite happy with the naming, open for suggestions.

[sammccall]

Very nice! Just want to give some thought to what these callsites look like.

clang/include/clang/Sema/Sema.h

3899 ↗

(On Diff #269218)

This definitely seems like something that makes sense to call in more places. Wonder if the API can be smoother.

Both of the examples are of the form: CorrectDelayedTypos, if invalid, rebuildTypos.
Are *all* likely cases of this form? (Could make it an optional part of CorrectDelayedTypos)
Are there any places where you want CorrectDelayedTypos and don't want rebuild? (Could make it an eventually-mandatory part of CorrectDelayedTypos)

3900 ↗

(On Diff #269218)

given current API, I'd suggest RecoverFromUncorrectedTypos or so

[hokein]

clang/include/clang/Sema/Sema.h

3899 ↗

(On Diff #269218)

Are *all* likely cases of this form? (Could make it an optional part of CorrectDelayedTypos)

I think so (at least for the moment).

Are there any places where you want CorrectDelayedTypos and don't want rebuild? (Could make it an eventually-mandatory part of CorrectDelayedTypos)

yeah, I think the eventual goal is to make the rebuild as the default fallback when typo correction fails, but it is still too far away to do it at the moment -- many testcases are broken (some are crashes, some are diagnostics improvements) when turning on recovery-ast by default.

I suppose we'd need to fix that case by case until it is ready, given this situation, adding an option parameter (recoveryUncorrectedTypos) to CorrectDelayedTyposInExpr API seems a feasible solution.

[hokein]

Make recovery fallback as part of the CorrectDelayedTyposInExpr.

[sammccall]

LG once the overload set is fixed

clang/include/clang/Sema/Sema.h

3876 ↗

(On Diff #270746)

I think it's too confusing to have multiple overloads with different subsets of the parameters possible and also in different orders :-(

If there's nothing better, you could replace this with two overloads

CorrectDelayedTyposInExpr(ER, Filter)
CorrectDelayedTyposInExpr(ER, bool, Filter)

clang/lib/Sema/SemaDecl.cpp

12042–12043

could consider splitting this change out of the refactoring (or vice versa), up to you

clang/lib/Sema/SemaExprCXX.cpp

8318 ↗

(On Diff #270746)

/*RecoverUncorrectedTypos=*/

[hokein]

clang/include/clang/Sema/Sema.h

3876 ↗

(On Diff #270746)

This method doesn't have too many usages, I'd just remove it, which leaves us two overloads (one for Expr, the other one for ExprResult).

clang/lib/Sema/SemaDecl.cpp

12042–12043

split the refactoring change in https://reviews.llvm.org/D82047

[sammccall]

(this looks reasonable now but it'd be nice to have a rebased diff!)

[hokein]

rebase to head.

clang/lib/Sema/SemaDecl.cpp

if (!VDecl->isInvalidDecl()) {
     MultiExprArg Args = Init;
     if (CXXDirectInit)
       Args = MultiExprArg(CXXDirectInit->getExprs(),
                           CXXDirectInit->getNumExprs());
 
     // Try to correct any TypoExprs in the initialization arguments.
     for (size_t Idx = 0; Idx < Args.size(); ++Idx) {
       ExprResult Res = CorrectDelayedTyposInExpr(
-          Args[Idx], VDecl, /*RecoverUncorrectedTypos=*/false,
+          Args[Idx], VDecl, /*RecoverUncorrectedTypos=*/true,
           [this, Entity, Kind](Expr *E) {
             InitializationSequence Init(*this, Entity, Kind, MultiExprArg(E));
             return Init.Failed() ? ExprError() : E;
           });
       if (Res.isInvalid()) {
         VDecl->setInvalidDecl();
       } else if (Res.get() != Args[Idx]) {
         Args[Idx] = Res.get();
       }

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

void test_invalid_call(int s) {
   some_func(undef1, undef2+1);
 
   // CHECK:      BinaryOperator {{.*}} '<dependent type>' contains-errors '='
   // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
   // CHECK-NEXT: `-CallExpr {{.*}} '<dependent type>' contains-errors
   // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
   // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   s = some_func(undef1);
-  // CHECK: `-VarDecl {{.*}} invalid var 'int'
-  // FIXME: preserve the broken call.
+
+  // CHECK:     VarDecl {{.*}} var 'int'
+  // CHECK-NEXT: `-CallExpr {{.*}} '<dependent type>' contains-errors
+  // CHECK-NEXT:   |-UnresolvedLookupExpr {{.*}} 'some_func'
+  // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   int var = some_func(undef1);
 }
 
 int ambig_func(double);
 int ambig_func(float);
 
 // CHECK:     VarDecl {{.*}} ambig_call
 // CHECK-NEXT:  `-RecoveryExpr {{.*}} 'int' contains-errors
void InvalidInitalizer(int x) {
   Bar b5 = Bar(invalid());
   // CHECK:     `-VarDecl {{.*}} b6 'Bar'
   // CHECK-NEXT: `-CXXUnresolvedConstructExpr {{.*}} 'Bar' contains-errors 'Bar'
   // CHECK-NEXT:  `-InitListExpr {{.*}} contains-errors
   // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   // CHECK-NEXT:     `-UnresolvedLookupExpr {{.*}} 'invalid'
   Bar b6 = Bar{invalid()};
 
-  // CHECK:     `-RecoveryExpr {{.*}} 'Bar' contains-errors
+  // CHECK:     RecoveryExpr {{.*}} 'Bar' contains-errors
   // CHECK-NEXT:  `-IntegerLiteral {{.*}} 'int' 1
   Bar(1);
+
+  // CHECK:     `-VarDecl {{.*}} var1
+  // CHECK-NEXT: `-BinaryOperator {{.*}} '<dependent type>' contains-errors
+  // CHECK-NEXT:   |-RecoveryExpr {{.*}} '<dependent type>' contains-errors
+  // CHECK-NEXT:   `-IntegerLiteral {{.*}} 'int' 1
+  int var1 = undef + 1;
 }
 void InitializerForAuto() {
   // CHECK:     `-VarDecl {{.*}} invalid a 'auto'
   // CHECK-NEXT: `-RecoveryExpr {{.*}} '<dependent type>' contains-errors
   // CHECK-NEXT:   `-UnresolvedLookupExpr {{.*}} 'invalid'
   auto a = invalid();
 
   // CHECK:     `-VarDecl {{.*}} invalid b 'auto'

clang/test/SemaCXX/typo-correction-delayed.cpp

void test() {
 
   // expected-error@+2 {{use of undeclared identifier 'variableX'; did you mean 'variable'?}}
   // expected-error@+1 {{no member named 'getX' in 'PR21925::X'; did you mean 'get'?}}
   int x = variableX.getX();
 }
 }
 
 namespace PR21905 {
-int (*a) () = (void)Z;  // expected-error-re {{use of undeclared identifier 'Z'{{$}}}}
+int (*a)() = (void)Z; // expected-error-re {{use of undeclared identifier 'Z'{{$}}}} \
+                      // expected-error {{cannot initialize a variable of type 'int (*)()' with an rvalue of type 'void'}}
 }
 
 namespace PR21947 {
 int blue;  // expected-note {{'blue' declared here}}
 __typeof blur y;  // expected-error {{use of undeclared identifier 'blur'; did you mean 'blue'?}}
 }
 
 namespace PR22092 {