Add test for eval order for a+=b #79293
Merged
Conversation
This file contains 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
Havvy
added
the
T-compiler
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Yes, the order of evaluation *does* change depending on the types of the operands. Cursed, I know. I've elected to place this test into `expr/compound-assignment` creating both the `expr` directory and the `compound-assignment` directory. I plan in a future PR to also move the `if` directory and the loose `if` tests into `expr/if` and other similar cleanups of the `test/ui` directory. Future work: Test more than just `+=`, but all operators. I don't know if using a macro to generate these tests cases would be okay or not, but it'd be boilerplatey without it. I'm also confident you cannot change the evaluation order of one operator without changing all of them. Future work: Additionally, test more than just `i32 += i32` for the primitive version. I don't actually know the full set of primitive implementations, but I imagine there's enough to cause a combinatorial explosion with the previous future work item. Somewhere on the order of one to two hundred individual functions.
Havvy
force-pushed
the
test-eval-order-compound-assign
branch
from
bc3ef0d to
b6f9705
Compare
|
It seems good to add a test for this -- I suspect there's probably at least one already out there, due to NLL/MIR work, but I wasn't able to quickly find it so this seems fine. @bors r+ rollup |
|
📌 Commit b6f9705 has been approved by |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 22, 2020
Rollup of 10 pull requests Successful merges: - rust-lang#76941 (Add f{32,64}::is_subnormal) - rust-lang#77697 (Split each iterator adapter and source into individual modules) - rust-lang#78305 (Stabilize alloc::Layout const functions) - rust-lang#78608 (Stabilize refcell_take) - rust-lang#78793 (Clean up `StructuralEq` docs) - rust-lang#79267 (BTreeMap: address namespace conflicts) - rust-lang#79293 (Add test for eval order for a+=b) - rust-lang#79295 (BTreeMap: fix minor testing mistakes in rust-lang#78903) - rust-lang#79297 (BTreeMap: swap the names of NodeRef::new and Root::new_leaf) - rust-lang#79299 (Stabilise `then`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
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.
Yes, the order of evaluation does change depending on the types of
the operands. Cursed, I know.
I've elected to place this test into
expr/compound-assignmentcreatingboth the
exprdirectory and thecompound-assignmentdirectory. Iplan in a future PR to also move the
ifdirectory and the looseiftests into
expr/ifand other similar cleanups of thetest/uidirectory.
Future work: Test more than just
+=, but all operators. I don't knowif using a macro to generate these tests cases would be okay or not,
but it'd be boilerplatey without it. I'm also confident you cannot
change the evaluation order of one operator without changing all of
them.
Future work: Additionally, test more than just
i32 += i32for theprimitive version. I don't actually know the full set of primitive
implementations, but I imagine there's enough to cause a combinatorial
explosion with the previous future work item. Somewhere on the order of
one to two hundred individual functions.