hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.
RecoveryExpr was always lvalue, but it is wrong if we use it to model
broken function calls, function call expression has more compliated rules:
- a call to a function whose return type is an lvalue reference yields an
lvalue;
- a call to a function whose return type is an rvalue reference yields an
xvalue;
- a call to a function whose return type is nonreference type yields a prvalue;
This patch makes the recovery-expr align with the function call if it is
modeled a broken call.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D83201
Files:
clang/include/clang/AST/Expr.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/Expr.cpp
clang/lib/AST/ExprClassification.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/AST/ast-dump-recovery.cpp
clang/test/SemaCXX/recovery-expr-type.cpp
Index: clang/test/SemaCXX/recovery-expr-type.cpp
===================================================================
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -62,3 +62,9 @@
// expected-note {{in instantiation of member function}} \
// expected-note {{in call to}}
}
+
+// verify no assertion failure on violating value category.
+namespace test4 {
+int&&f(int); // expected-note {{candidate function not viable}}
+int&& k = f(); // expected-error {{no matching function for call}}
+}
Index: clang/test/AST/ast-dump-recovery.cpp
===================================================================
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -4,7 +4,6 @@
int some_func(int *);
// CHECK: VarDecl {{.*}} invalid_call
-// CHECK-NEXT: `-ImplicitCastExpr {{.*}} 'int' contains-errors
// CHECK-NEXT: `-RecoveryExpr {{.*}} 'int' contains-errors
// CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
// CHECK-NEXT: `-IntegerLiteral {{.*}} 123
@@ -34,7 +33,6 @@
int ambig_func(float);
// CHECK: VarDecl {{.*}} ambig_call
-// CHECK-NEXT: `-ImplicitCastExpr {{.*}} 'int' contains-errors
// CHECK-NEXT: `-RecoveryExpr {{.*}} 'int' contains-errors
// CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'ambig_func'
// CHECK-NEXT: `-IntegerLiteral {{.*}} 123
@@ -211,3 +209,16 @@
} NoCrashOnInvalidInitList = {
.abc = nullptr,
};
+
+// Verify the value category of recovery expression.
+int f1(int);
+int& f2(int);
+int&& f3(int);
+void ValueCategory() {
+ // CHECK: RecoveryExpr {{.*}} 'int' contains-errors
+ f1(); // call to a function (nonreference return type) yields a prvalue (not print by default)
+ // CHECK: RecoveryExpr {{.*}} 'int' contains-errors lvalue
+ f2(); // call to a function (lvalue reference return type) yields an lvalue.
+ // CHECK: RecoveryExpr {{.*}} 'int' contains-errors xvalue
+ f3(); // call to a function (rvalue reference return type) yields an xvalue.
+}
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12818,7 +12818,7 @@
auto ConsiderCandidate = [&](const OverloadCandidate &Candidate) {
if (!Candidate.Function)
return;
- QualType T = Candidate.Function->getCallResultType();
+ QualType T = Candidate.Function->getReturnType();
if (T.isNull())
return;
if (!Result)
@@ -12938,8 +12938,15 @@
// 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));
+ auto ReturnType = chooseRecoveryType(*CandidateSet, Best);
+ auto Recovery = SemaRef.CreateRecoveryExpr(
+ Fn->getBeginLoc(), RParenLoc, SubExprs,
+ ReturnType.isNull()
+ ? ReturnType
+ // set the call result type.
+ : ReturnType.getNonLValueExprType(SemaRef.getASTContext()),
+ ReturnType.isNull() ? VK_LValue : Expr::getValueKindForType(ReturnType));
+ return Recovery;
}
static void markUnaddressableCandidatesUnviable(Sema &S,
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19209,7 +19209,8 @@
}
ExprResult Sema::CreateRecoveryExpr(SourceLocation Begin, SourceLocation End,
- ArrayRef<Expr *> SubExprs, QualType T) {
+ ArrayRef<Expr *> SubExprs, QualType T,
+ ExprValueKind VK) {
// 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.
@@ -19219,8 +19220,10 @@
if (isSFINAEContext())
return ExprError();
- if (T.isNull() || !Context.getLangOpts().RecoveryASTType)
+ 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);
+ VK = VK_LValue;
+ }
+ return RecoveryExpr::Create(Context, T, VK, Begin, End, SubExprs);
}
Index: clang/lib/AST/ExprClassification.cpp
===================================================================
--- clang/lib/AST/ExprClassification.cpp
+++ clang/lib/AST/ExprClassification.cpp
@@ -130,7 +130,6 @@
case Expr::UnresolvedLookupExprClass:
case Expr::UnresolvedMemberExprClass:
case Expr::TypoExprClass:
- case Expr::RecoveryExprClass:
case Expr::DependentCoawaitExprClass:
case Expr::CXXDependentScopeMemberExprClass:
case Expr::DependentScopeDeclRefExprClass:
@@ -145,6 +144,9 @@
case Expr::OMPIteratorExprClass:
return Cl::CL_LValue;
+ case Expr::RecoveryExprClass:
+ return ClassifyExprValueKind(Lang, E, E->getValueKind());
+
// C99 6.5.2.5p5 says that compound literals are lvalues.
// In C++, they're prvalue temporaries, except for file-scope arrays.
case Expr::CompoundLiteralExprClass:
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -4777,9 +4777,10 @@
return OriginalTy;
}
-RecoveryExpr::RecoveryExpr(ASTContext &Ctx, QualType T, SourceLocation BeginLoc,
- SourceLocation EndLoc, ArrayRef<Expr *> SubExprs)
- : Expr(RecoveryExprClass, T, VK_LValue, OK_Ordinary), BeginLoc(BeginLoc),
+RecoveryExpr::RecoveryExpr(ASTContext &Ctx, QualType T, ExprValueKind VK,
+ SourceLocation BeginLoc, SourceLocation EndLoc,
+ ArrayRef<Expr *> SubExprs)
+ : Expr(RecoveryExprClass, T, VK, OK_Ordinary), BeginLoc(BeginLoc),
EndLoc(EndLoc), NumExprs(SubExprs.size()) {
assert(!T.isNull());
assert(llvm::all_of(SubExprs, [](Expr* E) { return E != nullptr; }));
@@ -4789,12 +4790,12 @@
}
RecoveryExpr *RecoveryExpr::Create(ASTContext &Ctx, QualType T,
- SourceLocation BeginLoc,
+ ExprValueKind VK, SourceLocation BeginLoc,
SourceLocation EndLoc,
ArrayRef<Expr *> SubExprs) {
void *Mem = Ctx.Allocate(totalSizeToAlloc<Expr *>(SubExprs.size()),
alignof(RecoveryExpr));
- return new (Mem) RecoveryExpr(Ctx, T, BeginLoc, EndLoc, SubExprs);
+ return new (Mem) RecoveryExpr(Ctx, T, VK, BeginLoc, EndLoc, SubExprs);
}
RecoveryExpr *RecoveryExpr::CreateEmpty(ASTContext &Ctx, unsigned NumSubExprs) {
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3952,7 +3952,8 @@
/// Attempts to produce a RecoveryExpr after some AST node cannot be created.
ExprResult CreateRecoveryExpr(SourceLocation Begin, SourceLocation End,
ArrayRef<Expr *> SubExprs,
- QualType T = QualType());
+ QualType T = QualType(),
+ ExprValueKind VK = VK_LValue);
ObjCInterfaceDecl *getObjCInterfaceDecl(IdentifierInfo *&Id,
SourceLocation IdLoc,
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -6228,7 +6228,7 @@
class RecoveryExpr final : public Expr,
private llvm::TrailingObjects<RecoveryExpr, Expr *> {
public:
- static RecoveryExpr *Create(ASTContext &Ctx, QualType T,
+ static RecoveryExpr *Create(ASTContext &Ctx, QualType T, ExprValueKind VK,
SourceLocation BeginLoc, SourceLocation EndLoc,
ArrayRef<Expr *> SubExprs);
static RecoveryExpr *CreateEmpty(ASTContext &Ctx, unsigned NumSubExprs);
@@ -6255,8 +6255,9 @@
}
private:
- RecoveryExpr(ASTContext &Ctx, QualType T, SourceLocation BeginLoc,
- SourceLocation EndLoc, ArrayRef<Expr *> SubExprs);
+ RecoveryExpr(ASTContext &Ctx, QualType T, ExprValueKind VK,
+ 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
