[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5653d127d7c4: [docs] Give the reason why the support for coroutine is partial (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115778/new/ https://reviews.llvm.org/D115778 Files: clang/www/cxx_status.html Index: clang/www/cxx_status.html === --- clang/www/cxx_status.html +++ clang/www/cxx_status.html @@ -1199,7 +1199,13 @@ Coroutines https://wg21.link/p0912r5;>P0912R5 - Partial + +Partial + The optimizer does not yet handle TLS with + `__attribute__((const))` attribute correctly. There can be issues where the + coroutine may resume on a different thread. This feature requires further + analysis of the C++ Standard to determine what work is necessary for conformance. + Index: clang/www/cxx_status.html === --- clang/www/cxx_status.html +++ clang/www/cxx_status.html @@ -1199,7 +1199,13 @@ Coroutines https://wg21.link/p0912r5;>P0912R5 - Partial + +Partial + The optimizer does not yet handle TLS with + `__attribute__((const))` attribute correctly. There can be issues where the + coroutine may resume on a different thread. This feature requires further + analysis of the C++ Standard to determine what work is necessary for conformance. + ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial
ChuanqiXu updated this revision to Diff 394529. ChuanqiXu added a comment. Address comments. Thanks for reviewing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115778/new/ https://reviews.llvm.org/D115778 Files: clang/www/cxx_status.html Index: clang/www/cxx_status.html === --- clang/www/cxx_status.html +++ clang/www/cxx_status.html @@ -1199,7 +1199,13 @@ Coroutines https://wg21.link/p0912r5;>P0912R5 - Partial + +Partial + The optimizer does not yet handle TLS with + `__attribute__((const))` attribute correctly. There can be issues where the + coroutine may resume on a different thread. This feature requires further + analysis of the C++ Standard to determine what work is necessary for conformance. + Index: clang/www/cxx_status.html === --- clang/www/cxx_status.html +++ clang/www/cxx_status.html @@ -1199,7 +1199,13 @@ Coroutines https://wg21.link/p0912r5;>P0912R5 - Partial + +Partial + The optimizer does not yet handle TLS with + `__attribute__((const))` attribute correctly. There can be issues where the + coroutine may resume on a different thread. This feature requires further + analysis of the C++ Standard to determine what work is necessary for conformance. + ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a minor wording fix. Thank you for this! Comment at: clang/www/cxx_status.html:1206-1207 + `__attribute__((const))` attribute correctly. There can be issues where the + coroutine may resume on a different thread. Also there are works reamined to + check through the sections on coroutines in the C++ standard. + Fixes typo and grammar but hopefully gets the same point across. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115778/new/ https://reviews.llvm.org/D115778 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial
ChuanqiXu added inline comments. Comment at: clang/www/cxx_status.html:1267-1270 +(12): The optimizer couldn't handle TLS with + `__attribute__((const))` attribute correctly. It implies that there would + be problems if the coroutine is possible to resume on a different thread. + aaron.ballman wrote: > Also, when moving it to be a detail of the current row, I'd drop the `(12): ` > from the text. > > I think we should also add some words to the effect of "This feature was > implemented based on the Coroutines TS and requires further analysis to > determine what support remains to be added." This should hopefully make it > clear that the list of issues found is not exhaustive and that investigative > work is still needed. The current implementation have some minor differences with coroutines TS already. So I chose the wording Richard gives in the mailing list. I think it is proper. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115778/new/ https://reviews.llvm.org/D115778 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial
ChuanqiXu updated this revision to Diff 394527. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115778/new/ https://reviews.llvm.org/D115778 Files: clang/www/cxx_status.html Index: clang/www/cxx_status.html === --- clang/www/cxx_status.html +++ clang/www/cxx_status.html @@ -1199,7 +1199,13 @@ Coroutines https://wg21.link/p0912r5;>P0912R5 - Partial + +Partial + The optimizer does not yet handle TLS with + `__attribute__((const))` attribute correctly. There can be issues where the + coroutine may resume on a different thread. Also there are works reamined to + check through the sections on coroutines in the C++ standard. + Index: clang/www/cxx_status.html === --- clang/www/cxx_status.html +++ clang/www/cxx_status.html @@ -1199,7 +1199,13 @@ Coroutines https://wg21.link/p0912r5;>P0912R5 - Partial + +Partial + The optimizer does not yet handle TLS with + `__attribute__((const))` attribute correctly. There can be issues where the + coroutine may resume on a different thread. Also there are works reamined to + check through the sections on coroutines in the C++ standard. + ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial
aaron.ballman added a comment. Thanks for this! Comment at: clang/www/cxx_status.html:1202 https://wg21.link/p0912r5;>P0912R5 - Partial + Partial (12) Rather than following this approach, it'd be nice to have the information provided directly inline, as done in: https://github.com/llvm/llvm-project/blob/main/clang/www/cxx_status.html#L916 Comment at: clang/www/cxx_status.html:1267-1270 +(12): The optimizer couldn't handle TLS with + `__attribute__((const))` attribute correctly. It implies that there would + be problems if the coroutine is possible to resume on a different thread. + Also, when moving it to be a detail of the current row, I'd drop the `(12): ` from the text. I think we should also add some words to the effect of "This feature was implemented based on the Coroutines TS and requires further analysis to determine what support remains to be added." This should hopefully make it clear that the list of issues found is not exhaustive and that investigative work is still needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115778/new/ https://reviews.llvm.org/D115778 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D115778: [docs] Give the reason why the support for coroutine is partial
ChuanqiXu created this revision. ChuanqiXu added reviewers: aaron.ballman, jyknight. ChuanqiXu added a project: clang. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. See https://lists.llvm.org/pipermail/cfe-dev/2021-December/069608.html. We couldn't mark coroutine as done now due to there to critical bugs. And as Aaron suggests, it would be better to give the reason in the docs. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D115778 Files: clang/www/cxx_status.html Index: clang/www/cxx_status.html === --- clang/www/cxx_status.html +++ clang/www/cxx_status.html @@ -1199,7 +1199,7 @@ Coroutines https://wg21.link/p0912r5;>P0912R5 - Partial + Partial (12) @@ -1263,6 +1263,12 @@ -std=c++20, but can be enabled with -fchar8_t. + +(12): The optimizer couldn't handle TLS with + `__attribute__((const))` attribute correctly. It implies that there would + be problems if the coroutine is possible to resume on a different thread. + + C++2b implementation status Index: clang/www/cxx_status.html === --- clang/www/cxx_status.html +++ clang/www/cxx_status.html @@ -1199,7 +1199,7 @@ Coroutines https://wg21.link/p0912r5;>P0912R5 - Partial + Partial (12) @@ -1263,6 +1263,12 @@ -std=c++20, but can be enabled with -fchar8_t. + +(12): The optimizer couldn't handle TLS with + `__attribute__((const))` attribute correctly. It implies that there would + be problems if the coroutine is possible to resume on a different thread. + + C++2b implementation status ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
