Skip to content
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

Add C4868 to _STL_DISABLED_WARNINGS #4067

Conversation

tiagomacarios
Copy link
Member

@tiagomacarios tiagomacarios commented Oct 3, 2023

warning C4868: compiler may not enforce left-to-right evaluation order in braced initializer list

Office is seeing instances of C4868 being emitted inside of the STL. Example:
xutility(5637) : warning C4868: compiler may not enforce left-to-right evaluation order in braced initializer list

After discussing the issue with Stephan he suggested adding the supression to the STL itself:

You can non-intrusively add this to _STL_EXTRA_DISABLED_WARNINGS.
We would also consider a PR to add it to _STL_DISABLED_WARNINGS for
everyone (as we do globally suppress a number of /Wall warnings there).
We basically never have such ordering assumptions in the STL, so this
warning provides no value to us.

[warning C4868: compiler may not enforce left-to-right evaluation order in braced initializer list](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-c4868?view=msvc-170)

Office is seeing instances of C4868 being emitted inside of the STL. Example:
`xutility(5637) : warning C4868: compiler may not enforce left-to-right evaluation order in braced initializer list`

After discussing the issue with Stephan he suggested adding the supression to the STL itself:
```
You can non-intrusively add this to _STL_EXTRA_DISABLED_WARNINGS.
We would also consider a PR to add it to _STL_DISABLED_WARNINGS for
everyone (as we do globally suppress a number of /Wall warnings there).
We basically never have such ordering assumptions in the STL, so this
warning provides no value to us.
```
@tiagomacarios tiagomacarios requested a review from a team as a code owner October 3, 2023 15:26
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Oct 3, 2023
stl/inc/yvals_core.h Outdated Show resolved Hide resolved
stl/inc/yvals_core.h Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

Thanks!

@CaseyCarter @strega-nil-ms I pushed changes after you approved. I had forgotten that <execution> locally disabled this warning, but I have a heuristic "does anything else mention this" when making changes of this nature, so searching the codebase turned up these mentions of 4868. (Ctrl+Shift+F is incredibly powerful 🪄) At that point I noticed that <execution>'s comments said "(/Wall)", which in theory I could have noticed by looking at <yvals_core.h> alone but I missed that on first glance too. Just explaining my thought process here!

@StephanTLavavej StephanTLavavej self-assigned this Oct 3, 2023
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit b1d81af into microsoft:main Oct 4, 2023
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks for silencing this annoying warning and congratulations on your first microsoft/STL commit! 🎉 🚀 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants