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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D108696#2983021 <https://reviews.llvm.org/D108696#2983021>, @ldionne wrote:

> Based on the commit description, I don't understand this change at all. Why do we want to tweak the name lookup just for `std::coroutine`? Yes, we do have an action item to finish coroutines in libc++ already, and I'd love to see a patch that does that, but I don't think that mandates changing Clang.
>
> The rollout plan for coroutines should be:
>
> 1. Make sure we implement coroutines fully
> 2. Duplicate it all into namespace `std`
> 3. In two LLVM releases, remove all the coroutines stuff in `std::experimental`.
>
> I'm going to revert this for now on the basis that it breaks libc++ CI. Let's have a discussion about the above if you think I'm mistaken or if I'm misunderstanding what this patch does.

Many thanks for reverting this!

> Why do we want to tweak the name lookup just for `std::coroutine`? Yes, we do have an action item to finish coroutines in libc++ already

This is due to a practice that:

- People want to use clang but they prefer to use libstdc++ instead of libc++.
- So that they would implement a `coroutine.h` by coping from libc++. So that they could depend on libstdc++ still.

This is the reason that I introduce the warning in the compiler side instead of libc++. But if this warning matters, I am Ok to emit the warning in the compiler side.

Then let's talk about the plans to move coroutine components into std namespace:

In D108697 <https://reviews.llvm.org/D108697>, the successor of this patch, we had duplicated all the components into namespace `std`. (And I add an warning in `std::experimental` by #warning directive, I am not sure if this warning may break the CI)
Then here is the problem, we must commit this before D108697 <https://reviews.llvm.org/D108697>, otherwise the compiler wouldn't search coroutine components in `std`  namespace.
The solution I got now is that I should remove the warning message in this patch. Then the compiler would still try to lookup in  `std::experimental` namespace without warning if it can't find things in `std` namespace.
Do you @ldionne  @Quuxplusone think this solution workable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108696