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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
