This is an archive of the discontinued LLVM Phabricator instance.

Paths

Table of Contentst

-
clang/www/
-
www/
1/4
cxx_status.html

Differential D115778

[docs] Give the reason why the support for coroutine is partial
ClosedPublic

Authored by ChuanqiXu on Dec 14 2021, 6:23 PM.

Download Raw Diff

Details

Reviewers

aaron.ballman
jyknight

Commits

rG5653d127d7c4: [docs] Give the reason why the support for coroutine is partial

Summary

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.

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

ChuanqiXu requested review of this revision.Dec 14 2021, 6:23 PM

ChuanqiXu created this revision.

Herald added a subscriber: cfe-commits. · View Herald TranscriptDec 14 2021, 6:23 PM

Harbormaster completed remote builds in B139352: Diff 394439.Dec 14 2021, 7:01 PM

Thanks for this!

clang/www/cxx_status.html
1202–1208	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
1273–1276	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.

Address comments.

ChuanqiXu added inline comments.Dec 15 2021, 4:41 AM

clang/www/cxx_status.html
1273–1276	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.

LGTM aside from a minor wording fix. Thank you for this!

clang/www/cxx_status.html
1206–1207	Fixes typo and grammar but hopefully gets the same point across.

This revision is now accepted and ready to land.Dec 15 2021, 4:45 AM

Address comments. Thanks for reviewing.

This revision was landed with ongoing or failed builds.Dec 15 2021, 5:02 AM

Closed by commit rG5653d127d7c4: [docs] Give the reason why the support for coroutine is partial (authored by ChuanqiXu). · Explain Why

This revision was automatically updated to reflect the committed changes.

ChuanqiXu added a commit: rG5653d127d7c4: [docs] Give the reason why the support for coroutine is partial.

Harbormaster completed remote builds in B139414: Diff 394529.Dec 15 2021, 5:30 AM

Revision Contents

Path

Size

clang/

www/

cxx_status.html

8 lines

Diff 394530

clang/www/cxx_status.html

Show First 20 Lines • Show All 1,193 Lines • ▼ Show 20 Lines </tr>

</tr> </tr>

</tr> </tr>

<td>Coroutines</td> <td>Coroutines</td>

<td class="partial" align="center">Partial</td> <td class="partial" align="center">

<details><summary>Partial</summary>

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. <br \>This feature requires further

analysis of the C++ Standard to determine what work is necessary for conformance.

aaron.ballmanUnsubmitted

Not Done

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

+ coroutine may resume on a different thread.<br \>This feature requires further

+ analysis of the C++ Standard to determine what work is necessary for conformance.

</details></td>

Fixes typo and grammar but hopefully gets the same point across.

aaron.ballman: Fixes typo and grammar but hopefully gets the same point across.

</details></td>

aaron.ballmanUnsubmitted

Not Done

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

aaron.ballman: Rather than following this approach, it'd be nice to have the information provided directly…

</tr> </tr>

<td>Deprecate <tt>a[b,c]</tt></td> <td>Deprecate <tt>a[b,c]</tt></td>

<td class="full" align="center">Clang 9</td> <td class="full" align="center">Clang 9</td>

</tr> </tr>

▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Lines

</table> </table>

<p> <p>

<span id="p0482">(11): Prior to Clang 8, this feature is not enabled by <span id="p0482">(11): Prior to Clang 8, this feature is not enabled by

<tt>-std=c++20</tt>, but can be enabled with <tt>-fchar8_t</tt>. <tt>-std=c++20</tt>, but can be enabled with <tt>-fchar8_t</tt>.

</span> </span>

</p> </p>

</details> </details>

<h2 id="cxx23">C++2b implementation status</h2> <h2 id="cxx23">C++2b implementation status</h2>

<p>Clang has support for some of the features of the C++ standard following <p>Clang has support for some of the features of the C++ standard following

aaron.ballmanUnsubmitted

Not Done

</p>

<p>

- <span id="CoroPartialReason">(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.

+ <span id="CoroPartialReason">(12): 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.

</span>

</p>

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.

aaron.ballman: Also, when moving it to be a detail of the current row, I'd drop the `(12): ` from the text. I…

ChuanqiXuAuthorUnsubmitted

Done

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.

ChuanqiXu: The current implementation have some minor differences with coroutines TS already. So I chose…

C++20, informally referred to as C++2b.</p> C++20, informally referred to as C++2b.</p>

<p>You can use Clang in C++2b mode with the <code>-std=c++2b</code> option.</p> <p>You can use Clang in C++2b mode with the <code>-std=c++2b</code> option.</p>

<summary>List of features and minimum Clang version with support</summary> <summary>List of features and minimum Clang version with support</summary>

▲ Show 20 Lines • Show All 234 Lines • Show Last 20 Lines

This is an archive of the discontinued LLVM Phabricator instance.

[docs] Give the reason why the support for coroutine is partialClosedPublic

Details

Diff Detail

Event Timeline

Revision Contents

Diff 394530

clang/www/cxx_status.html

[docs] Give the reason why the support for coroutine is partial
ClosedPublic