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

Fri Jul 28 02:36:55 PDT 2023

ilya-biryukov added subscribers: thieta, tstellar.
ilya-biryukov added a comment.

@aaron.ballman the internal `-cc1` flag is good enough for us and should aleviate most of the concerns in this thread. We could also live without a libc++ change.
I suggest to move forward with `-cc1` flag. Would that be okay or do you feel this does not address some of the concerns mentioned in the thread?

@thieta, @tstellar would you be willing to cherry-pick a flag to disable coroutines to a release branch?
There are two potential forms, in case the answers differ:

- user-facing `clang -fno-coroutines` flag,
- internal `clang -cc1 -fno-coroutines`.

In D156247#4540228 <https://reviews.llvm.org/D156247#4540228>, @Mordante wrote:

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

I am sorry for sending the patch late, we have not thought about it before. I have already mentioned that before in the thread.
We could have foreseen the need for this change before, but we did not and there is nothing I can do to fix it now.

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

As mentioned before, we feel this would give us substantial benefits when rolling out C++20.
I acknowledge that coroutines are good enough for most people and those bugs may not be a problem. We are overly cautious, our bar is higher and we would not to ship coroutines with those bugs (at the very least we'd like to have an option to bail out).

> - 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,

I have asked the release managers above.

> - a libc++ patch that works with `-fno-coroutines`, which I would be happy to review.

I will check what needs to be done for libc++ here, I suspect adding `#error` inside `<coroutine>` if the feature macro is not defined should be enough.
It might not be necessary if we use a `-cc1` flag.

In D156247#4541003 <https://reviews.llvm.org/D156247#4541003>, @ChuanqiXu wrote:

>> We should reach out to some GCC folks to see if this is an oversight in their documentation or not.
>
> I've sent a mail: https://gcc.gnu.org/pipermail/gcc/2023-July/242159.html

Thanks, there is now a response in the thread saying that documentation intentionally leaves out most `-fno-foo` flags and GCC supports `-fno-coroutines`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156247