Quuxplusone added inline comments.
================
Comment at: clang/lib/Sema/SemaCoroutine.cpp:896
+ if (S.CanPerformCopyInitialization(Entity, &AsRvalue))
+ return true;
+ } else if (auto *FTD = dyn_cast<FunctionTemplateDecl>(D)) {
----------------
aaronpuchert wrote:
> Overlad resolution can actually still fail if there is a viable candidate,
> namely when there are multiple candidates and none is better than all others.
> It's a bit weird though to fall back to lvalue parameter then as if nothing
> happened.
That is an interesting point! I had not considered it during
[P1155](https://wg21.link/p1155r2). I imagine that it might make implementation
of [P1155](https://wg21.link/p1155r2)'s new logic more difficult.
GCC 8 (but not trunk) implements the behavior I would expect to see for
derived-to-base conversions: https://godbolt.org/z/fj_lNw
C++ always treats "an embarrassment of riches" equivalently to a "famine";
overload resolution //can// fail due to ambiguity just as easily as it can fail
due to no candidates at all. I agree it's "a bit weird," but it would be vastly
weirder if C++ did anything //different// from its usual behavior in this case.
I'm still amenable to the idea that `co_return` should simply not do the
copy-elision or implicit-move optimizations at all. I wish I knew some
use-cases for `co_return`, so that we could see if the optimization is useful
to anyone.
================
Comment at: clang/test/SemaCXX/coroutine-rvo.cpp:65
+ NoCopyNoMove value;
+ co_return value;
+}
----------------
@aaronpuchert wrote:
> Given the complexities of this implementation, I'm beginning to doubt whether
> implicit moves make sense for `co_return` at all. Since there can never be
> any kind of RVO, why not always require an explicit `std::move`? Implicit
> moves on return were introduced because an explicit move would inhibit NRVO,
> and without move there wouldn't be a graceful fallback for implementations
> that don't have NRVO.
I still think that it makes sense to do implicit-move on //any// control-flow
construct that "exits" the current function. Right now that's `return` and
`throw`, and they both do implicit-move (albeit inconsistently with plenty of
implementation divergence documented in [P1155](http://wg21.link/p1155r2)).
C++2a adds `co_return` as another way to "exit." I think it would be
un-graceful and inconsistent to permit `return p` but require `co_return
std::move(p)`.
Now, I do think that C++2a Coroutines are needlessly complicated, which makes
the implementation needlessly complicated. You've noticed that to codegen
`t.return_value(x)` requires not just overload resolution, but actually
figuring out whether `task::return_value` is a function at all — it might be a
static data member of function-pointer type, or something [even
wilder](https://quuxplusone.github.io/blog/2019/09/18/cppcon-whiteboard-puzzle/).
But eliminating implicit-move won't make that complexity go away, will it?
Doing implicit-move just means doing the required overload resolution twice
instead of once. That //should// be easy, compared to the rest of the mess.
That said, I would be happy to start a thread among you, me, David Stone, and
the EWG mailing list to consider removing the words "or `co_return`" from
[class.copy.elision/3](http://eel.is/c++draft/class.copy.elision#3.1). Say the
word.
[EDIT: aw shoot, I never submitted this comment last time]
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68845/new/
https://reviews.llvm.org/D68845
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
