This is an archive of the discontinued LLVM Phabricator instance.

[Coroutines] [libcxx] Move coroutine component out of experimental namespace
AbandonedPublic

Authored by ChuanqiXu on Aug 23 2021, 11:30 PM.

Details

Summary

Now in libcxx and clang, all the coroutine components are defined in std::experimental namespace.
And now the coroutine TS is merged into C++20. So in the working draft like N4892, we could find the coroutine components is defined in std namespace instead of std::experimental namespace. And the coroutine support in clang seems to be relatively stable. So I think it may be suitable to move the coroutine component into the experiment namespace now.

To make clang compatible with legacy uses, the use of coroutine in std::experimental namespace is remained.

Test Plan: check-all

Diff Detail

Event Timeline

ChuanqiXu created this revision.Aug 23 2021, 11:30 PM
ChuanqiXu requested review of this revision.Aug 23 2021, 11:30 PM
ChuanqiXu updated this revision to Diff 368305.Aug 24 2021, 1:53 AM

fix tests in clang-tools-extra.

Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2021, 1:53 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

I am not familiar with the process of when to move something out of experimental, but I do wonder how this is normally done so that people who uses coroutines can have a smooth migration?
I assume that this is going to be a breaking change that existing code using coroutine will need to be updated and no longer compatible with old versions.

ChuanqiXu updated this revision to Diff 368551.Aug 24 2021, 9:44 PM
ChuanqiXu edited the summary of this revision. (Show Details)
  • Make clang support looking up in std::experimental namespace.

I am not familiar with the process of when to move something out of experimental, but I do wonder how this is normally done so that people who uses coroutines can have a smooth migration?
I assume that this is going to be a breaking change that existing code using coroutine will need to be updated and no longer compatible with old versions.

Thanks for reporting this. Now the clang could still look up in std::experimental namespace of once it fails to look up in std namespace. So now it wouldn't be a break change if the user upgrade the version clang only.

Fix test: libcxx/utils/generate_header_tests.py

Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2021, 11:20 PM
ChuanqiXu abandoned this revision.Aug 25 2021, 5:41 AM

Oh, I find why the CI still fails. Since the compiler used to compile libcxx in the CI doesn't contain the change in this patch! So it would still lookup coroutine components in std::experimental namespace. So here are the failures. To avoid the problems, I had split this patch into D108696 and D108697, which contains the change in clang part and libcxx part respectively.