[PATCH] D146187: [docs] Update the status for coroutines

This revision was automatically updated to reflect the committed changes. Closed by commit rG8894fe7a6f3e: [docs] Update the status for coroutines (authored by ChuanqiXu). Changed prior to commit: https://reviews.llvm.org/D146187?vs=505979&id=506821#toc Repository: rG LLVM Github Monorepo C

[PATCH] D146187: [docs] Update the status for coroutines

ChuanqiXu added a comment. > Okay, that pretty much paints us into a corner, I guess. If we don't define > it any longer, we break existing (working) uses of the feature on Windows, > but we defined it prematurely. In this case, let's leave the macro defined so > we don't break existing uses --

[PATCH] D146187: [docs] Update the status for coroutines

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146187/new/ https://reviews.llvm.org/D146187 ___ cfe-commits

[PATCH] D146187: [docs] Update the status for coroutines

aaron.ballman added a comment. In D146187#4205271 , @ChuanqiXu wrote: >> Ugh, that does sort of change the calculus for whether we do or don't claim >> support on Windows. If removing the definition of that macro on Windows >> causes significant code br

[PATCH] D146187: [docs] Update the status for coroutines

ChuanqiXu added a comment. > Ugh, that does sort of change the calculus for whether we do or don't claim > support on Windows. If removing the definition of that macro on Windows > causes significant code breakage, that would be a reason we should leave it > defined. But do we have evidence of

[PATCH] D146187: [docs] Update the status for coroutines

bruno added a comment. > ... I think in general we want feature testing macros to be treated as our > signal to the user that we think something is complete, not that we think > something is in progress but pretty usable. +1 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146187/new/ h

[PATCH] D146187: [docs] Update the status for coroutines

aaron.ballman added a comment. In D146187#4201910 , @cor3ntin wrote: > In D146187#4201905 , @aaron.ballman > wrote: > >> In D146187#4201223 , @ChuanqiXu >> wrote: >> >>>

[PATCH] D146187: [docs] Update the status for coroutines

aaron.ballman added a comment. In D146187#4201271 , @ChuanqiXu wrote: >> Should we also be updating InitPreprocessor.cpp at the same time, for >> non-Windows targets? > > I didn't address this since we didn't do this before. (Defining feature macro > ac

[PATCH] D146187: [docs] Update the status for coroutines

cor3ntin added a comment. In D146187#4201905 , @aaron.ballman wrote: > In D146187#4201223 , @ChuanqiXu > wrote: > >>> but we don't define __cpp_­impl_­coroutine: >>> http://eel.is/c++draft/tab:cpp.predefined.ft

[PATCH] D146187: [docs] Update the status for coroutines

aaron.ballman added a comment. In D146187#4201223 , @ChuanqiXu wrote: >> but we don't define __cpp_­impl_­coroutine: >> http://eel.is/c++draft/tab:cpp.predefined.ft > > We defined `__cpp_­impl_­coroutine`. And `__cpp_coroutines` is for > Coroutines TS.

[PATCH] D146187: [docs] Update the status for coroutines

ChuanqiXu added a comment. > Should we also be updating InitPreprocessor.cpp at the same time, for > non-Windows targets? I didn't address this since we didn't do this before. (Defining feature macro according to the targets). Also I feel it may be bad for windows users. Since currently the co

[PATCH] D146187: [docs] Update the status for coroutines

ChuanqiXu added a comment. > but we don't define __cpp_­impl_­coroutine: > http://eel.is/c++draft/tab:cpp.predefined.ft We defined `__cpp_­impl_­coroutine`. And `__cpp_coroutines` is for Coroutines TS. I forgot to remove this too. I'll handle it in a separate patch. CHANGES SINCE LAST ACTION

[PATCH] D146187: [docs] Update the status for coroutines

ChuanqiXu updated this revision to Diff 505979. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146187/new/ https://reviews.llvm.org/D146187 Files: clang/docs/ReleaseNotes.rst clang/www/cxx_status.html Index: clang/www/cxx_status.html ==

[PATCH] D146187: [docs] Update the status for coroutines

aaron.ballman added a comment. Should we also be updating InitPreprocessor.cpp at the same time, for non-Windows targets? I see we define: // TS features. if (LangOpts.Coroutines) Builder.defineMacro("__cpp_coroutines", "201703L"); but we don't define `__cpp_­impl_­coroutine`: http://e

[PATCH] D146187: [docs] Update the status for coroutines

ChuanqiXu created this revision. ChuanqiXu added reviewers: aaron.ballman, bruno, clang-language-wg. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. According to the discussion in https://discourse.ll