[PATCH] [coroutines] Rename driver flag -fcoroutines to -fcoroutines-ts

Gor Nishanov via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 30 14:32:14 PDT 2016


Uploaded to phabricator as https://reviews.llvm.org/D25130

__cpp_coroutines stays as it is.

On Fri, Sep 30, 2016 at 10:11 AM, Gor Nishanov <gornishanov at gmail.com>
wrote:

> I noticed that other TSes has experimental in their SD-6 macro name,
> hence, I changed it to match the concepts TS macro
> __cpp_experimental_concepts.
> We are reviewing the wording coming Monday and can tweak the SD-6 macro as
> needed.
>
> Also, Richard mentioned in the feedback that:
>
> >> However, we *shouldn't be* defining this until we have a complete
> implementation. (Code using this for a feature test wants to test whether
> the feature works, not just whether it's enabled on the command line.)
>
> Richard, do you want us to take out __cpp_coroutines completely until all
> of the parts are pushed upstream?
>
> On Thu, Sep 29, 2016 at 9:59 PM, Eric Fiselier <eric at efcs.ca> wrote:
>
>> On Sep 29, 2016 8:23 PM, "Gor Nishanov" <gornishanov at gmail.com> wrote:
>> >
>> > You beat me to it, Eric. :) I'll add mine for review, too. Let's see
>> which one Richard will respond :) .
>> >
>> > 1. Remove __has_feature
>> > 2. Rename fcoroutines => fcoroutines_TS
>> > 3. Rename __cpp_coroutines => __cpp_experimental_coroutines
>>
>> The TS spec uses __cpp_coroutines. What's the reason for changing it?
>>
>> >
>> > Since phabricator is down, here is a handy diff on a github
>> > https://github.com/GorNishanov/clang/commit/e129083a73cf82e0
>> bcea0817045ae6baaadccbb7
>> >
>> >
>> >
>> >
>> > On Thu, Sep 29, 2016 at 6:22 PM, Eric Fiselier <eric at efcs.ca> wrote:
>> >>
>> >> I've attached an updated patch which addresses the comments.
>> >>
>> >> 1. Remove __has_feature changes.
>> >> 2. Rename OPT_fcoroutines -> OPT_fcoroutines_TS.
>> >>
>> >> /Eric
>> >>
>> >> On Thu, Sep 29, 2016 at 6:58 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>> >>>
>> >>> +def fcoroutines : Flag <["-"], "fcoroutines-ts">, Group<f_Group>, +
>> Flags<[DriverOption, CC1Option]>, + HelpText<"Enable support for the C++
>> Coroutines TS">; +def fno_coroutines : Flag <["-"], "fno-coroutines-ts">,
>> Group<f_Group>,
>> >>>
>> >>> These should be named fcoroutines_ts, fno_coroutines_ts (see comment
>> at the top of the file for the naming scheme).
>> >>>
>> >>> + .Case("coroutines", LangOpts.Coroutines)
>> >>>
>> >>> We should use the SD-6 macro name (__cpp_coroutines) rather than
>> __has_feature for new features that are covered by SD-6. However, we
>> shouldn't be defining this until we have a complete implementation. (Code
>> using this for a feature test wants to test whether the feature works, not
>> just whether it's enabled on the command line.)
>> >>>
>> >>> On Thu, Sep 29, 2016 at 5:45 PM, Gor Nishanov <gornishanov at gmail.com>
>> wrote:
>> >>>>
>> >>>> Let's see if renaming the attachment to *.txt helps.
>> >>>>
>> >>>> On Thu, Sep 29, 2016 at 5:42 PM, Gor Nishanov <gornishanov at gmail.com>
>> wrote:
>> >>>>>
>> >>>>> Currently the -fcoroutines flag is a CC1 only flag. It really
>> should be both a Driver and CC1 flag. This patch fixes the option and adds
>> tests for the new options.
>> >>>>>
>> >>>>> Also adds a __has_feature for coroutines.
>> >>>>>
>> >>>>> Patch is mostly by Eric Fiselier
>> >>>>> .
>> >>>>> Meticulous and painstaking extraction from the larger coroutine
>> branch by Gor Nishanov
>> >>>>>
>> >>>>> P.S.
>> >>>>>
>> >>>>> Switching to lowercase [coroutines] tag in the title, as most of
>> the coroutine commits in cfe were done with lowercase tag.
>> >>>>
>> >>>>
>> >>>
>> >>
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160930/c537647d/attachment.html>


More information about the cfe-commits mailing list