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 <gornisha...@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 <e...@efcs.ca> wrote: > >> On Sep 29, 2016 8:23 PM, "Gor Nishanov" <gornisha...@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 <e...@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 <rich...@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 <gornisha...@gmail.com> >> wrote: >> >>>> >> >>>> Let's see if renaming the attachment to *.txt helps. >> >>>> >> >>>> On Thu, Sep 29, 2016 at 5:42 PM, Gor Nishanov <gornisha...@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. >> >>>> >> >>>> >> >>> >> >> >> > >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
