[clang] 96a5cff - [AST][RecoveryExpr] Fix the value category for recovery expr.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-07-08T13:55:07+02:00
New Revision: 96a5cfff208d8e86a598e64412d9ef5dde0f9c9e

URL: https://github.com/llvm/llvm-project/commit/96a5cfff208d8e86a598e64412d9ef5dde0f9c9e
DIFF: https://github.com/llvm/llvm-project/commit/96a5cfff208d8e86a598e64412d9ef5dde0f9c9e.diff

LOG: [AST][RecoveryExpr] Fix the value category for recovery expr.

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.

Differential revision: https://reviews.llvm.org/D83201

Added: 
    

Modified: 
    clang/lib/AST/Expr.cpp
    clang/lib/AST/ExprClassification.cpp
    clang/lib/Sema/SemaOverload.cpp
    clang/test/AST/ast-dump-recovery.cpp
    clang/test/SemaCXX/recovery-expr-type.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 96a70307235b..343a271c3394 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4779,8 +4779,10 @@ QualType OMPArraySectionExpr::getBaseOriginalType(const Expr *Base) {
 
 RecoveryExpr::RecoveryExpr(ASTContext &Ctx, QualType T, SourceLocation BeginLoc,
                            SourceLocation EndLoc, ArrayRef<Expr *> SubExprs)
-    : Expr(RecoveryExprClass, T, VK_LValue, OK_Ordinary), BeginLoc(BeginLoc),
-      EndLoc(EndLoc), NumExprs(SubExprs.size()) {
+    : Expr(RecoveryExprClass, T.getNonReferenceType(),
+           T->isDependentType() ? VK_LValue : getValueKindForType(T),
+           OK_Ordinary),
+      BeginLoc(BeginLoc), EndLoc(EndLoc), NumExprs(SubExprs.size()) {
   assert(!T.isNull());
   assert(llvm::all_of(SubExprs, [](Expr* E) { return E != nullptr; }));
 

diff  --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index 42873d090ffa..31aa734ffedb 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -130,7 +130,6 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) {
   case Expr::UnresolvedLookupExprClass:
   case Expr::UnresolvedMemberExprClass:
   case Expr::TypoExprClass:
-  case Expr::RecoveryExprClass:
   case Expr::DependentCoawaitExprClass:
   case Expr::CXXDependentScopeMemberExprClass:
   case Expr::DependentScopeDeclRefExprClass:
@@ -276,6 +275,7 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) {
       return Cl::CL_PRValue;
     }
 
+  case Expr::RecoveryExprClass:
   case Expr::OpaqueValueExprClass:
     return ClassifyExprValueKind(Lang, E, E->getValueKind());
 

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 599e81d1b4d0..8635397f4806 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -12818,7 +12818,7 @@ static QualType chooseRecoveryType(OverloadCandidateSet &CS,
   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)

diff  --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp
index e2a715130628..740864a26481 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/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(double);
 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 @@ struct {
 } NoCrashOnInvalidInitList = {
   .abc = nullptr,
 };
+
+// Verify the value category of recovery expression.
+int prvalue(int);
+int &lvalue(int);
+int &&xvalue(int);
+void ValueCategory() {
+  // CHECK:  RecoveryExpr {{.*}} 'int' contains-errors
+  prvalue(); // call to a function (nonreference return type) yields a prvalue (not print by default)
+  // CHECK:  RecoveryExpr {{.*}} 'int' contains-errors lvalue
+  lvalue(); // call to a function (lvalue reference return type) yields an lvalue.
+  // CHECK:  RecoveryExpr {{.*}} 'int' contains-errors xvalue
+  xvalue(); // call to a function (rvalue reference return type) yields an xvalue.
+}

diff  --git a/clang/test/SemaCXX/recovery-expr-type.cpp b/clang/test/SemaCXX/recovery-expr-type.cpp
index df40e5cd6021..6cd79326e8c3 100644
--- a/clang/test/SemaCXX/recovery-expr-type.cpp
+++ b/clang/test/SemaCXX/recovery-expr-type.cpp
@@ -62,3 +62,9 @@ constexpr auto x2 = AA<int>::foo2(); // expected-error {{be initialized by a con
                                      // 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}}
+}