[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

[Lewis Baker via Phabricator via cfe-commits]

lewissbaker added a comment.

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>

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>`.

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?

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.


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

https://reviews.llvm.org/D108696