[PATCH] D156247: [Clang] Add a warning on uses of coroutine keywords

Thu Jul 27 13:10:50 PDT 2023

Mordante added a comment.

In D156247#4538626 <https://reviews.llvm.org/D156247#4538626>, @aaron.ballman wrote:

> I would like explicit buy-in from the libc++ folks on the idea of adding `-fno-coroutines` as they're going to be impacted by Clang supporting such an option (and they may have additional testing requirements to ensure that libc++ works reasonably well when coroutines are disabled).

Thanks for the adding libc++ to the review @aaron.ballman!

If some concerns with this patch

1. I share @philnik's concern that it adds a new configuration we need to test.
2. My biggest concern is the timing of this patch. The date of the branching is know upfront and well communicated by the release managers. This patch was created after the LLVM-17 branch was created. For libc++ this patch would be a new feature which has no patch at the moment. This means we're asked to add a new feature after RC1. In libc++ we normally avoid adding new features in the release branch.
3. I don't see a consensus from the Clang developers to add this option. I also see no consensus coroutines are "broken". I agree if we were to do something the `-fno-coroutines` would be the way to go.

I do not speak for Apple, but I know the beta's for the new XCode are available since June and I have not heard from @ldionne (who works at Apple) there were concerns with coroutines. @ldionne is out of the office at the moment. So even when we add this option now it will not be available in the next XCode. (Unless Apple backports it.)

@ilya-biryukov if you feel strongly about pursuing this patch I would like to see:

- a consensus of the Clang developers this is really needed,
- a pre-approval by the release managers to apply these changes to the release branch,
- a libc++ patch that works with `-fno-coroutines`, which I would be happy to review.

@ilya-biryukov the next time it would be really great to have such intrusive changes up for review well before the release branch is created. That gives the LLVM community time review this without the need to rush it. For me it would have been a lot easier to accept such a patch a month ago than it's now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156247