[AST][RecoveryExpr] Fix a crash on undeduced type.

[hokein]

Summary

We should not capture the type if the function return type is undeduced.

[sammccall]

clang/lib/Sema/SemaOverload.cpp

12883–12888

Wouldn't we be better to handle undeduced type here rather than in the loop?

Not sure if it practically matters, but given: auto f(); double f(int); f(0,0);
the code in this patch will discard auto and yield double, while treating this as multiple candidates --> unknown return type seems more correct.

[hokein]

clang/lib/Sema/SemaOverload.cpp

12883–12888

I feel like yield double is better in your given example -- in this case, double is the only candidate, so it is reasonable to preserve the double type.

[sammccall]

clang/lib/Sema/SemaOverload.cpp

12883–12888

There are two overloads, one with double and one with an unknown type. They're both candidates for the function being called, and I'm not sure we have any reason to believe the first is more likely.

Two counterarguments I can think of:

• the unknown type is probably also double, as overloads tend to have the same return type
• the cost of being wrong isn't so high, so it's OK to not be totally sure

And maybe there are others... these are probably OK justifications but subtle so I'd like to see a comment tothat effect.

[hokein]

address comments.

[hokein]

clang/lib/Sema/SemaOverload.cpp

12883–12888

thanks, I think the behavior you suggest is more reasonable -- in the given example, go-to-def will yield two candidates, it is wired to catch one of the return type (not the other).

Addressed it.

[sammccall]

LG, thanks!

clang/lib/Sema/SemaOverload.cpp

if (!Result)
     for (const auto &C : CS)
       if (C.Viable)
         ConsiderCandidate(C);
   // Finally, consider all candidates.
   if (!Result)
     for (const auto &C : CS)
       ConsiderCandidate(C);
 
-  return Result.getValueOr(QualType());
+  if (!Result)
+    return QualType();
+  auto Value = Result.getValue();
+  if (Value.isNull() || Value->isUndeducedType())
+    return QualType();
+  return Value;
 }
 
 /// FinishOverloadedCallExpr - given an OverloadCandidateSet, builds and returns
 /// the completed call expression. If overload resolution fails, emits
 /// diagnostics and returns ExprError()
 static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
                                            UnresolvedLookupExpr *ULE,
                                            SourceLocation LParenLoc,

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

void test(int x) {
   foo->func(x);
 }
 
 // CHECK:     |-AlignedAttr {{.*}} alignas
 // CHECK-NEXT:| `-RecoveryExpr {{.*}} contains-errors
 // CHECK-NEXT:|   `-UnresolvedLookupExpr {{.*}} 'invalid'
 struct alignas(invalid()) Aligned {};
 
+auto f();
+int f(double);
+// CHECK:      VarDecl {{.*}} unknown_type_call 'int'
+// CHECK-NEXT: `-RecoveryExpr {{.*}} '<dependent type>'
+int unknown_type_call = f(0, 0);
+
 void InvalidInitalizer(int x) {
   struct Bar { Bar(); };
   // CHECK:     `-VarDecl {{.*}} a1 'Bar'
   // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors
   // CHECK-NEXT:  `-IntegerLiteral {{.*}} 'int' 1
   Bar a1(1);
   // CHECK:     `-VarDecl {{.*}} a2 'Bar'
   // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors

clang/test/SemaCXX/recovery-expr-type.cpp

 }
 
 // verify the secondary diagnostic "cannot initialize" is emitted.
 namespace test8 {
 typedef int arr[];
 int v = arr(); // expected-error {{array types cannot be value-initialized}} \
                   expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'test8::arr'}}
 }
+
+namespace test9 {
+auto f(); // expected-note {{candidate function not viable}}
+// verify no crash on evaluating the size of undeduced auto type.
+static_assert(sizeof(f(1)), ""); // expected-error {{no matching function for call to 'f'}}
+}