hokein updated this revision to Diff 262874.
hokein marked an inline comment as done.
hokein added a comment.

add a missing test file during the rebase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79160/new/

https://reviews.llvm.org/D79160

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/CodeCompletion/member-access.cpp
  clang/test/Index/getcursor-recovery.cpp

Index: clang/test/Index/getcursor-recovery.cpp
===================================================================
--- clang/test/Index/getcursor-recovery.cpp
+++ clang/test/Index/getcursor-recovery.cpp
@@ -2,15 +2,24 @@
 int foo(int, double);
 int x;
 
-void testTypedRecoveryExpr() {
-  // Inner foo() is a RecoveryExpr, outer foo() is an overloaded call.
-  foo(x, foo(x));
+void testTypedRecoveryExpr1() {
+  // Inner bar() is an unresolved overloaded call, outer foo() is an overloaded call.
+  foo(x, bar(x));
 }
-// RUN: c-index-test -cursor-at=%s:7:3 %s -Xclang -frecovery-ast | FileCheck -check-prefix=OUTER-FOO %s
+// RUN: c-index-test -cursor-at=%s:7:3 %s -Xclang -frecovery-ast -Xclang -frecovery-ast-type | FileCheck -check-prefix=OUTER-FOO %s
 // OUTER-FOO: OverloadedDeclRef=foo[2:5, 1:5]
-// RUN: c-index-test -cursor-at=%s:7:7 %s -Xclang -frecovery-ast | FileCheck -check-prefix=OUTER-X %s
+// RUN: c-index-test -cursor-at=%s:7:7 %s -Xclang -frecovery-ast -Xclang -frecovery-ast-type | FileCheck -check-prefix=OUTER-X %s
 // OUTER-X: DeclRefExpr=x:3:5
-// RUN: c-index-test -cursor-at=%s:7:10 %s -Xclang -frecovery-ast | FileCheck -check-prefix=INNER-FOO %s
-// INNER-FOO: OverloadedDeclRef=foo[2:5, 1:5]
-// RUN: c-index-test -cursor-at=%s:7:14 %s -Xclang -frecovery-ast | FileCheck -check-prefix=INNER-X %s
+// RUN: c-index-test -cursor-at=%s:7:10 %s -Xclang -frecovery-ast -Xclang -frecovery-ast-type | FileCheck -check-prefix=INNER-FOO %s
+// INNER-FOO: OverloadedDeclRef=bar
+// RUN: c-index-test -cursor-at=%s:7:14 %s -Xclang -frecovery-ast -Xclang -frecovery-ast-type | FileCheck -check-prefix=INNER-X %s
 // INNER-X: DeclRefExpr=x:3:5
+
+void testTypedRecoveryExpr2() {
+  // Inner foo() is a RecoveryExpr (with int type), outer foo() is a valid "foo(int, int)" call.
+  foo(x, foo(x));
+}
+// RUN: c-index-test -cursor-at=%s:20:3 %s -Xclang -frecovery-ast -Xclang -frecovery-ast-type | FileCheck -check-prefix=TEST2-OUTER %s
+// TEST2-OUTER: DeclRefExpr=foo:1:5
+// RUN: c-index-test -cursor-at=%s:20:10 %s -Xclang -frecovery-ast -Xclang -frecovery-ast-type | FileCheck -check-prefix=TEST2-INNER %s
+// TEST2-INNER: OverloadedDeclRef=foo[2:5, 1:5]
Index: clang/test/CodeCompletion/member-access.cpp
===================================================================
--- clang/test/CodeCompletion/member-access.cpp
+++ clang/test/CodeCompletion/member-access.cpp
@@ -273,3 +273,12 @@
 // RUN: --implicit-check-not="[#char#]operator=("
 // CHECK-OPER: [#int#]operator=(
 
+struct S { int member; };
+S overloaded(int);
+S overloaded(double);
+void foo() {
+  // No overload matches, but we have recovery-expr with the correct type.
+  overloaded().
+}
+// RUN: not %clang_cc1 -fsyntax-only -frecovery-ast -frecovery-ast-type -code-completion-at=%s:281:16 %s -o - | FileCheck -check-prefix=CHECK-RECOVERY %s
+// CHECK-RECOVERY: [#int#]member
Index: clang/test/AST/ast-dump-recovery.cpp
===================================================================
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -1,12 +1,13 @@
-// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -fcxx-exceptions -std=gnu++17 -frecovery-ast -ast-dump %s | FileCheck -strict-whitespace %s
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -fcxx-exceptions -std=gnu++17 -frecovery-ast -frecovery-ast-type -ast-dump %s | FileCheck -strict-whitespace %s
 // RUN: not %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -fcxx-exceptions -std=gnu++17 -fno-recovery-ast -ast-dump %s | FileCheck --check-prefix=DISABLED -strict-whitespace %s
 
 int some_func(int *);
 
 // CHECK:     VarDecl {{.*}} invalid_call
-// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
-// CHECK-NEXT:  |-UnresolvedLookupExpr {{.*}} 'some_func'
-// CHECK-NEXT:  `-IntegerLiteral {{.*}} 123
+// CHECK-NEXT: `-ImplicitCastExpr {{.*}} 'int' contains-errors
+// CHECK-NEXT:  `-RecoveryExpr {{.*}} 'int' contains-errors
+// CHECK-NEXT:    |-UnresolvedLookupExpr {{.*}} 'some_func'
+// CHECK-NEXT:    `-IntegerLiteral {{.*}} 123
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int invalid_call = some_func(123);
 
@@ -14,14 +15,15 @@
 int ambig_func(float);
 
 // CHECK:     VarDecl {{.*}} ambig_call
-// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
-// CHECK-NEXT:  |-UnresolvedLookupExpr {{.*}} 'ambig_func'
-// CHECK-NEXT:  `-IntegerLiteral {{.*}} 123
+// CHECK-NEXT: `-ImplicitCastExpr {{.*}} 'int' contains-errors
+// CHECK-NEXT:  `-RecoveryExpr {{.*}} 'int' contains-errors
+// CHECK-NEXT:    |-UnresolvedLookupExpr {{.*}} 'ambig_func'
+// CHECK-NEXT:    `-IntegerLiteral {{.*}} 123
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int ambig_call = ambig_func(123);
 
 // CHECK:     VarDecl {{.*}} unresolved_call1
-// CHECK-NEXT:`-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:`-RecoveryExpr {{.*}} '<dependent type>' contains-errors
 // CHECK-NEXT:  `-UnresolvedLookupExpr {{.*}} 'bar'
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int unresolved_call1 = bar();
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12776,6 +12776,42 @@
   return false;
 }
 
+// Guess at what the return type for an unresolvable overload should be.
+static QualType chooseRecoveryType(OverloadCandidateSet &CS,
+                                   OverloadCandidateSet::iterator *Best) {
+  llvm::Optional<QualType> Result;
+  // Adjust Type after seeing a candidate.
+  auto ConsiderCandidate = [&](const OverloadCandidate &Candidate) {
+    if (!Candidate.Function)
+      return;
+    QualType T = Candidate.Function->getCallResultType();
+    if (T.isNull())
+      return;
+    if (!Result)
+      Result = T;
+    else if (Result != T)
+      Result = QualType();
+  };
+
+  // Look for an unambiguous type from a progressively larger subset.
+  // e.g. if types disagree, but all *viable* overloads return int, choose int.
+  //
+  // First, consider only the best candidate.
+  if (Best && *Best != CS.end())
+    ConsiderCandidate(**Best);
+  // Next, consider only viable candidates.
+  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());
+}
+
 /// FinishOverloadedCallExpr - given an OverloadCandidateSet, builds and returns
 /// the completed call expression. If overload resolution fails, emits
 /// diagnostics and returns ExprError()
@@ -12865,8 +12901,11 @@
   }
   }
 
-  // Overload resolution failed.
-  return ExprError();
+  // Overload resolution failed, try to recover.
+  SmallVector<Expr *, 8> SubExprs = {Fn};
+  SubExprs.append(Args.begin(), Args.end());
+  return SemaRef.CreateRecoveryExpr(Fn->getBeginLoc(), RParenLoc, SubExprs,
+                                    chooseRecoveryType(*CandidateSet, Best));
 }
 
 static void markUnaddressableCandidatesUnviable(Sema &S,
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -18950,7 +18950,7 @@
 }
 
 ExprResult Sema::CreateRecoveryExpr(SourceLocation Begin, SourceLocation End,
-                                    ArrayRef<Expr *> SubExprs) {
+                                    ArrayRef<Expr *> SubExprs, QualType T) {
   // FIXME: enable it for C++, RecoveryExpr is type-dependent to suppress
   // bogus diagnostics and this trick does not work in C.
   // FIXME: use containsErrors() to suppress unwanted diags in C.
@@ -18960,5 +18960,8 @@
   if (isSFINAEContext())
     return ExprError();
 
-  return RecoveryExpr::Create(Context, Begin, End, SubExprs);
+  if (T.isNull() || !Context.getLangOpts().RecoveryASTType)
+    // We don't know the concrete type, fallback to dependent type.
+    T = Context.DependentTy;
+  return RecoveryExpr::Create(Context, T, Begin, End, SubExprs);
 }
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16434,7 +16434,7 @@
   }
 
   QualType EltTy = Context.getBaseElementType(T);
-  if (!EltTy->isDependentType()) {
+  if (!EltTy->isDependentType() && !EltTy->containsErrors()) {
     if (RequireCompleteSizedType(Loc, EltTy,
                                  diag::err_field_incomplete_or_sizeless)) {
       // Fields of incomplete type force their record to be invalid.
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2893,6 +2893,8 @@
     Diags.Report(diag::warn_fe_concepts_ts_flag);
   Opts.RecoveryAST =
       Args.hasFlag(OPT_frecovery_ast, OPT_fno_recovery_ast, false);
+  Opts.RecoveryASTType =
++      Args.hasFlag(OPT_frecovery_ast_type, OPT_fno_recovery_ast_type, false);
   Opts.HeinousExtensions = Args.hasArg(OPT_fheinous_gnu_extensions);
   Opts.AccessControl = !Args.hasArg(OPT_fno_access_control);
   Opts.ElideConstructors = !Args.hasArg(OPT_fno_elide_constructors);
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -4681,25 +4681,25 @@
   return OriginalTy;
 }
 
-RecoveryExpr::RecoveryExpr(ASTContext &Ctx, SourceLocation BeginLoc,
+RecoveryExpr::RecoveryExpr(ASTContext &Ctx, QualType T, SourceLocation BeginLoc,
                            SourceLocation EndLoc, ArrayRef<Expr *> SubExprs)
-    : Expr(RecoveryExprClass, Ctx.DependentTy, VK_LValue, OK_Ordinary),
-      BeginLoc(BeginLoc), EndLoc(EndLoc), NumExprs(SubExprs.size()) {
-#ifndef NDEBUG
+    : Expr(RecoveryExprClass, T, VK_LValue, OK_Ordinary), BeginLoc(BeginLoc),
+      EndLoc(EndLoc), NumExprs(SubExprs.size()) {
+  assert(!T.isNull());
   for (auto *E : SubExprs)
     assert(E != nullptr);
-#endif
 
   llvm::copy(SubExprs, getTrailingObjects<Expr *>());
   setDependence(computeDependence(this));
 }
 
-RecoveryExpr *RecoveryExpr::Create(ASTContext &Ctx, SourceLocation BeginLoc,
+RecoveryExpr *RecoveryExpr::Create(ASTContext &Ctx, QualType T,
+                                   SourceLocation BeginLoc,
                                    SourceLocation EndLoc,
                                    ArrayRef<Expr *> SubExprs) {
   void *Mem = Ctx.Allocate(totalSizeToAlloc<Expr *>(SubExprs.size()),
                            alignof(RecoveryExpr));
-  return new (Mem) RecoveryExpr(Ctx, BeginLoc, EndLoc, SubExprs);
+  return new (Mem) RecoveryExpr(Ctx, T, BeginLoc, EndLoc, SubExprs);
 }
 
 RecoveryExpr *RecoveryExpr::CreateEmpty(ASTContext &Ctx, unsigned NumSubExprs) {
Index: clang/lib/AST/ComputeDependence.cpp
===================================================================
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -485,9 +485,13 @@
 }
 
 ExprDependence clang::computeDependence(RecoveryExpr *E) {
+  // Mark the expression as value- and instantiation- dependent to reuse
+  // existing suppressions for dependent code, e.g. avoiding
+  // constant-evaluation.
   // FIXME: drop type+value+instantiation once Error is sufficient to suppress
   // bogus dianostics.
-  auto D = ExprDependence::TypeValueInstantiation | ExprDependence::Error;
+  auto D = toExprDependence(E->getType()->getDependence()) |
+           ExprDependence::ValueInstantiation | ExprDependence::Error;
   for (auto *S : E->subExpressions())
     D |= S->getDependence();
   return D;
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3890,7 +3890,8 @@
 
   /// Attempts to produce a RecoveryExpr after some AST node cannot be created.
   ExprResult CreateRecoveryExpr(SourceLocation Begin, SourceLocation End,
-                                ArrayRef<Expr *> SubExprs);
+                                ArrayRef<Expr *> SubExprs,
+                                QualType T = QualType());
 
   ObjCInterfaceDecl *getObjCInterfaceDecl(IdentifierInfo *&Id,
                                           SourceLocation IdLoc,
Index: clang/include/clang/Driver/CC1Options.td
===================================================================
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -569,6 +569,10 @@
   HelpText<"Preserve expressions in AST rather than dropping them when "
            "encountering semantic errors">;
 def fno_recovery_ast : Flag<["-"], "fno-recovery-ast">;
+def frecovery_ast_type : Flag<["-"], "frecovery-ast-type">,
+  HelpText<"Preserve the type for recovery expressions when possible "
+          "(experimental)">;
+def fno_recovery_ast_type : Flag<["-"], "fno-recovery-ast-type">;
 
 let Group = Action_Group in {
 
Index: clang/include/clang/Basic/LangOptions.def
===================================================================
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -149,6 +149,7 @@
 LANGOPT(DoubleSquareBracketAttributes, 1, 0, "'[[]]' attributes extension for all language standard modes")
 
 COMPATIBLE_LANGOPT(RecoveryAST, 1, 0, "Preserve expressions in AST when encountering errors")
+COMPATIBLE_LANGOPT(RecoveryASTType, 1, 0, "Preserve the type in recovery expressions")
 
 BENIGN_LANGOPT(ThreadsafeStatics , 1, 1, "thread-safe static initializers")
 LANGOPT(POSIXThreads      , 1, 0, "POSIX thread support")
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -6097,21 +6097,25 @@
 /// subexpressions of some expression that we could not construct and source
 /// range covered by the expression.
 ///
-/// For now, RecoveryExpr is type-, value- and instantiation-dependent to take
-/// advantage of existing machinery to deal with dependent code in C++, e.g.
-/// RecoveryExpr is preserved in `decltype(<broken-expr>)` as part of the
+/// By default, RecoveryExpr is type-, value- and instantiation-dependent to
+/// take advantage of existing machinery to deal with dependent code in C++,
+/// e.g. RecoveryExpr is preserved in `decltype(<broken-expr>)` as part of the
 /// `DependentDecltypeType`. In addition to that, clang does not report most
 /// errors on dependent expressions, so we get rid of bogus errors for free.
 /// However, note that unlike other dependent expressions, RecoveryExpr can be
 /// produced in non-template contexts.
+/// In addition, we will preserve the type in RecoveryExpr when the type is
+/// known, e.g. preserving the return type for a broken non-overloaded function
+/// call, a overloaded call where all candidates have the same return type.
 ///
 /// One can also reliably suppress all bogus errors on expressions containing
 /// recovery expressions by examining results of Expr::containsErrors().
 class RecoveryExpr final : public Expr,
                            private llvm::TrailingObjects<RecoveryExpr, Expr *> {
 public:
-  static RecoveryExpr *Create(ASTContext &Ctx, SourceLocation BeginLoc,
-                              SourceLocation EndLoc, ArrayRef<Expr *> SubExprs);
+  static RecoveryExpr *Create(ASTContext &Ctx, QualType T,
+                              SourceLocation BeginLoc, SourceLocation EndLoc,
+                              ArrayRef<Expr *> SubExprs);
   static RecoveryExpr *CreateEmpty(ASTContext &Ctx, unsigned NumSubExprs);
 
   ArrayRef<Expr *> subExpressions() {
@@ -6136,8 +6140,8 @@
   }
 
 private:
-  RecoveryExpr(ASTContext &Ctx, SourceLocation BeginLoc, SourceLocation EndLoc,
-               ArrayRef<Expr *> SubExprs);
+  RecoveryExpr(ASTContext &Ctx, QualType T, SourceLocation BeginLoc,
+               SourceLocation EndLoc, ArrayRef<Expr *> SubExprs);
   RecoveryExpr(EmptyShell Empty, unsigned NumSubExprs)
       : Expr(RecoveryExprClass, Empty), NumExprs(NumSubExprs) {}
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to