hokein added inline comments.

================
Comment at: clang/lib/Sema/SemaOverload.cpp:12944
+      Fn->getBeginLoc(), RParenLoc, SubExprs,
+      ReturnType.isNull()
+          ? ReturnType
----------------
sammccall wrote:
> 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.
> Hmm, this does seem simpler to me but it also seems that a few places 
> deliberately make this mapping between two related concepts explicit.

yeah, that's true for at least `CallExpr`. Expr provides a `setValueKind` 
method, ideally (if we enable the type propagation), we could call it to set 
the value category here. I think it is fine to leave it at recoveryexpr side -- 
our case is really simple.




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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83201



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to