ChuanqiXu added a comment. In D108696#3086011 <https://reviews.llvm.org/D108696#3086011>, @lewissbaker wrote:
> So am I correct in understanding that the main issue with the chicken/egg > problem for updating both the compiler to use the new stdlib facilities and > updating the stdlib facilities is that we don't want to issue warnings about > using `<experimental/coroutine>` and telling users to use `<coroutine>` > instead if there is no `<coroutine>` header. > > I wonder if a suitable transition approach here might be to have the compiler > do: > > - try lookup of std::coroutine_handle/coroutine_traits; if found try to > instantiate the expression (e.g. await-suspend) using the std:: type > - if not found then try lookup of > std::experimental::coroutine_handle/coroutine_traits; and if found try to > instantiate the expression using the std::experimental:: type; and if > successful, then; > - if std:: type was found but instantiating the expression failed, then > issue a warning about using deprecated std::experimental:: types > - otherwise if std:: type was not found, try to find <coroutine> header and > if present then issue a warning suggesting using <coroutine> instead of > <experimental/coroutine> > > Then you should be able to land the compiler change and it should continue to > have the same behaviour for existing code using std::experimental:: types > with the existing stdlib. > It is only once the stdlib changes that land which add the <coroutine> header > that users would start seeing the warnings about using <coroutine> instead of > <experimental/coroutine> It looks odd for me with the sentence: > otherwise if std:: type was not found, try to find <coroutine> header and if > present then issue a warning suggesting using <coroutine> instead of > <experimental/coroutine> If we could find the <coroutine> header, we should be able to find `std::coro*` types. Otherwise, the <coroutine> header is fake. My transition plan would be: - Land this. - Land D109433 <https://reviews.llvm.org/D109433>, it is the change who added <coroutine>. - Land another patch in clang to issue a warning for the deprecated use of <experimental/coroutine>. The reason why we need to split this with the first step is that we don't want to trigger the CI system for libc++. (It's the reason this patch is rejected before). - Remove <experimental/coroutine> in libcxx in LLVM15. - Remove the support for `std::experimental::coro*` in clang in LLVM16. > Note that there are still some potential breakages for code that could go on > here, however. > > Imagine that some header I included has been updated to use <coroutine> while > my current header is still using <experimental/coroutine> so that both are in > scope. > > I might have defined an awaitable type that looks like the following: > > c++ > #include <other/header.hpp> // which transitively includes <coroutine> > #include <experimental/coroutine> > > struct my_awaitable { > bool await_ready(); > void await_suspend(std::experimental::coroutine_handle<> coro); > void await_resume(); > }; > > In this case, the compiler would find the `std::coroutine_handle<>` and try > to instantiate the await-suspend expression with that type, which would fail > because `await_suspend()` is not callable with a `std::coroutine_handle`. > The compiler would need to fall back to instantiating the expression with > `std::experimental::coroutine_handle<P>`. My personal opinion is that this case should fail instead of requiring the compiler to fall back to instantiating with `std::experimental::coro*`. There are two reasons: - It is not easy to implement such fall back to instantiating logics. It would make the current search process much more complex. - It is not worth to do so in my opinion. Since <experimental/coroutine> is the deprecated use. It is odd for me that we need to do extra work to conform it. > There is also the variation where await_suspend() is a template function that > deduces the promise-type argument. > e.g. > > c++ > template<typename Promise> > void await_suspend(std::experimental::coroutine_handle<Promise> coro); > > There are also cases where the template parameter to await_suspend() is > unconstrained. > e.g. > > c++ > struct promise_type { > ... > std::experimental::coroutine_handle<> continuation; > }; > > struct my_awaitable { > bool await_ready(); > template<typename CoroHandle> > auto await_suspend(CoroHandle handle) { > coro.promise().continuation = handle; > return coro; > } > void await_resume(); > > std::experimental::coroutine_handle<promise_type> coro; > }; > > Such a type would successfully deduce the template parameter using > std::coroutine_handle and so the await-suspend expression would be valid, but > the instantiation of the await_suspend() body would fail as a > std::coroutine_handle<P> cannot be assigned to an lvalue of type > `std::experimentl::coroutine_handle<>`. This would not produce a > SFINAE-friendly error that the compiler could retry with std::experimental > types. > > I'm not sure if there's a great way of keeping such code working in a mixed > `<coroutine>` and `<experimental/coroutine>` world. > Maybe the cost of doing so is not worth the benefit? Thanks for offering these examples. It is valuable. But I think the root cause may be the mixed use for `<coroutine>` and `<experimental/coroutine>`. It is too bad. IMHO, it is not worth to do so. > The only way I can think of making this work is to just make > `std::experimental::*` an alias for `std::*`. > But that only works for `std::experimental::coroutine_handle`. It doesn't > work for `std::experimental::coroutine_traits` as you can't add > specialisations through an alias. Yes, the compiler would look up to `std::experimental::coroutine_traits` to get the namespace used. We have talked about the method to make `std::experimental::*` to be an alias for `std::*`. It should be done in libcxx part. And it is not valid since it doesn't obey the rule. Originally, I am changing <experimental/coroutine> directly. But then I am required to keep <experimental/coroutine> untouched. It makes sense to me so I didn't complain about that. IMO, the key point here is that if the mixed use of <coroutine> and <experimental/coroutine> need to be considered. I think we could get rid of it personally. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108696/new/ https://reviews.llvm.org/D108696 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
