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