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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/AST/ExprClassification.cpp:147
 
+  case Expr::RecoveryExprClass:
+    return ClassifyExprValueKind(Lang, E, E->getValueKind());
----------------
there's a block of cases with a similar implementation (near OpaqueValueKind), maybe move there


================
Comment at: clang/lib/Sema/SemaOverload.cpp:12944
+      Fn->getBeginLoc(), RParenLoc, SubExprs,
+      ReturnType.isNull()
+          ? ReturnType
----------------
hokein wrote:
> sammccall wrote:
> > here we're splitting the type (e.g. int&&) into a type + VK, and passing both to createrecoveryexpr.
> > 
> > Why not do that on recoveryexpr side? e.g. if we request a recoveryexpr of type int&, return an LValue recoveryexpr of type int?
> right, good idea, this is simpler. I was somehow taking `CallExpr` as a reference when writing this code.
Hmm, this does seem simpler to me but it also seems that a few places deliberately make this mapping between two related concepts explicit.
Maybe we should at least have a comment on createrecoveryexpr that the value category will be inferred from the (reference) type.


================
Comment at: clang/test/SemaCXX/recovery-expr-type.cpp:66
+
+namespace test4 {
+int &&f(int);  // expected-note {{candidate function not viable}}
----------------
I liked the comment explaining the purpose of the test (no crash for wrong value category)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83201