This revision was automatically updated to reflect the committed changes.
Closed by commit rG8222107aa924: [AST] Preserve the type in RecoveryExprs for
broken function calls. (authored by hokein).
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
@@ -16426,7 +16426,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
@@ -2890,6 +2890,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
@@ -565,6 +565,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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
