Appease MSVC Warning C4866: compiler may not enforce left-to-right evaluation order#5953
Merged
rwgk merged 4 commits intopybind:masterfrom Jan 8, 2026
Merged
Appease MSVC Warning C4866: compiler may not enforce left-to-right evaluation order#5953rwgk merged 4 commits intopybind:masterfrom
rwgk merged 4 commits intopybind:masterfrom
Conversation
Skylion007
approved these changes
Jan 7, 2026
Collaborator
Skylion007
left a comment
There was a problem hiding this comment.
Weird warning, but makes sense.
The previous comment referenced the MSVC warning but didn't explain why the code is structured as two statements. The revised comment clarifies the intent: fetching the value first ensures well-defined evaluation order.
Switch from crate-ci/typos to adhtruong/mirrors-typos because pre-commit autoupdate confuses tags in the upstream repo, selecting the mutable `v1` tag instead of pinned versions like `v1.41.0`. See crate-ci/typos#390
rwgk
approved these changes
Jan 7, 2026
Collaborator
rwgk
left a comment
There was a problem hiding this comment.
I added a couple trivial commits.
Will merge after I see that the CI has finished.
Collaborator
|
Hm, I don't remember seeing this failure (EDIT: it was resolved by a rerun): CI / 🐍 (macos-latest, 3.14t, -DCMAKE_CXX_STANDARD=20) / 🧪 (pull_request) |
XuehaiPan
commented
Jan 8, 2026
Comment on lines
+1084
to
+1087
| // Fetch value before indexing into kwargs to ensure well-defined | ||
| // evaluation order (MSVC C4866). | ||
| PyObject *const arg_in_arr = args_in_arr[n_args_in + i]; | ||
| kwargs[PyTuple_GET_ITEM(kwnames_in, i)] = arg_in_arr; |
Contributor
Author
There was a problem hiding this comment.
The warning still exists. The problem may be that the order of key casting and assignment is not well-defined.
C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-tsdbyp__\overlay\Lib\site-packages\pybind11\include\pybind11\pybind11.h(1087,1): error C2220: the following warning is treated as an error [C:\Users\runneradmin\AppData\Local\Temp\tmph2yqzrcl.build-temp\Release\src\_C.vcxproj]
C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-tsdbyp__\overlay\Lib\site-packages\pybind11\include\pybind11\pybind11.h(1087,1): warning C4866: compiler may not enforce left-to-right evaluation order for call to 'pybind11::detail::object_api<pybind11::handle>::operator[]' [C:\Users\runneradmin\AppData\Local\Temp\tmph2yqzrcl.build-temp\Release\src\_C.vcxproj]
Collaborator
|
Do you have a way to test locally first? |
Contributor
Author
Yes, working on it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This is a regression after #5948
Piggy-backed commit f16de66:
Switch
typospre-commit hook to a mirror repo (adhtruong/mirrors-typos) to fixpre-commit autoupdateselecting the mutablev1tag instead of pinned versions. See crate-ci/typos#390Suggested changelog entry: