-
Notifications
You must be signed in to change notification settings - Fork 263
Enable /await opt-out so users can manually enable /await:strict. #1015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The downside is that it means two libraries both compiled for C++17 with the same version of cppwinrt.exe but different versions of the NuGet package (or one compiled with the NuGet package and one without)[1] are now no longer ABI compatible. I think we need to make this opt-in rather than a change in default. (Or at least, if it's a change in default, a way to opt out.) [1] Example: A module that contains both C++/WinRT code (in C++17 mode and /await:strict) and C+/CX code (which uses C++17 and plain /await). We have a lot of modules like this internally inside Windows. |
|
My understanding is that CX code can also use /await:strict, My primary motivation for this is that without it you can't mix a static C++/WinRT library compiled with stdcpp20 within a dll project that uses C++/CX and therefore always uses stdcpp17. |
|
I am a fan of having opt-outs though so let me add that. |
|
Yeah, I am generally not a fan of things like NuGet packages forcing command line arguments on the project - if I was involved in the original design, I would have had the templates include |
Agreed, instead of adding /await:strict, I'm instead adding the option to opt-out of the current /await, so project maintainers can disable it and pass their own option. |
| <ClCompile> | ||
| <AdditionalOptions>%(AdditionalOptions) /bigobj</AdditionalOptions> | ||
| <AdditionalOptions Condition="'%(ClCompile.LanguageStandard)' == 'stdcpp17'">%(AdditionalOptions) /await</AdditionalOptions> | ||
| <AdditionalOptions Condition="'%(ClCompile.LanguageStandard)' == 'stdcpp17' And '$(CppWinRTEnableLegacyCoroutines)' != 'false'">%(AdditionalOptions) /await</AdditionalOptions> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just remove this, or does this affect existing projects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing it entirely could affect projects which relied on the implicit addition of /await.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, the logic is just throwing me but then msbuild has never made sense to me.
/await:strict is binary compatible with c++20 coroutines. Instead of adding new logic to the C++/WinRT build targets, allow opt-out of the current default so users can set this manually.
For details, see: https://docs.microsoft.com/en-us/cpp/build/reference/await-enable-coroutine-support?view=msvc-160 and https://devblogs.microsoft.com/cppblog/cpp20-coroutine-improvements-in-visual-studio-2019-version-16-11/
Fixes #1014