<div dir="ltr"><div>Uploaded to phabricator as <a href="https://reviews.llvm.org/D25130">https://reviews.llvm.org/D25130</a> <br><br></div>__cpp_coroutines stays as it is.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 30, 2016 at 10:11 AM, Gor Nishanov <span dir="ltr"><<a href="mailto:gornishanov@gmail.com" target="_blank">gornishanov@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div>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.</div><div>We are reviewing the wording coming Monday and can tweak the SD-6 macro as needed.</div></div><div><br></div><div>Also, Richard mentioned in the feedback that:</div><span class=""><div><br></div><div>>> However, we <b>shouldn't be</b> 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.)</div><div><br></div></span><div>Richard, do you want us to take out __cpp_coroutines completely until all of the parts are pushed upstream? <br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 29, 2016 at 9:59 PM, Eric Fiselier <span dir="ltr"><<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><p dir="ltr"></p>
<p dir="ltr">On Sep 29, 2016 8:23 PM, "Gor Nishanov" <<a href="mailto:gornishanov@gmail.com" target="_blank">gornishanov@gmail.com</a>> wrote:<br>
><br>
> You beat me to it, Eric. :) I'll add mine for review, too. Let's see which one Richard will respond :) .<br>
><br>
> 1. Remove __has_feature<br>
> 2. Rename fcoroutines => fcoroutines_TS<br>
> 3. Rename __cpp_coroutines => __cpp_experimental_coroutines</p>
</span><p dir="ltr">The TS spec uses __cpp_coroutines. What's the reason for changing it?<br></p><div class="m_-5824063488328580270HOEnZb"><div class="m_-5824063488328580270h5">
<p dir="ltr">><br>
> Since phabricator is down, here is a handy diff on a github<br>
> <a href="https://github.com/GorNishanov/clang/commit/e129083a73cf82e0bcea0817045ae6baaadccbb7" target="_blank">https://github.com/GorNishanov<wbr>/clang/commit/e129083a73cf82e0<wbr>bcea0817045ae6baaadccbb7</a><br>
><br>
><br>
><br>
><br>
> On Thu, Sep 29, 2016 at 6:22 PM, Eric Fiselier <<a href="mailto:eric@efcs.ca" target="_blank">eric@efcs.ca</a>> wrote:<br>
>><br>
>> I've attached an updated patch which addresses the comments.<br>
>><br>
>> 1. Remove __has_feature changes.<br>
>> 2. Rename OPT_fcoroutines -> OPT_fcoroutines_TS.<br>
>><br>
>> /Eric<br>
>><br>
>> On Thu, Sep 29, 2016 at 6:58 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br>
>>><br>
>>> +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>,<br>
>>><br>
>>> These should be named fcoroutines_ts, fno_coroutines_ts (see comment at the top of the file for the naming scheme).<br>
>>><br>
>>> + .Case("coroutines", LangOpts.Coroutines) <br>
>>><br>
>>> 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.)<br>
>>><br>
>>> On Thu, Sep 29, 2016 at 5:45 PM, Gor Nishanov <<a href="mailto:gornishanov@gmail.com" target="_blank">gornishanov@gmail.com</a>> wrote:<br>
>>>><br>
>>>> Let's see if renaming the attachment to *.txt helps.<br>
>>>><br>
>>>> On Thu, Sep 29, 2016 at 5:42 PM, Gor Nishanov <<a href="mailto:gornishanov@gmail.com" target="_blank">gornishanov@gmail.com</a>> wrote:<br>
>>>>><br>
>>>>> 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.<br>
>>>>><br>
>>>>> Also adds a __has_feature for coroutines.<br>
>>>>><br>
>>>>> Patch is mostly by Eric Fiselier<br>
>>>>> .<br>
>>>>> Meticulous and painstaking extraction from the larger coroutine branch by Gor Nishanov<br>
>>>>><br>
>>>>> P.S.<br>
>>>>><br>
>>>>> Switching to lowercase [coroutines] tag in the title, as most of the coroutine commits in cfe were done with lowercase tag.<br>
>>>><br>
>>>><br>
>>><br>
>><br>
></p>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>