tests/ui: A New Order [23/N]
#143298
tests/ui: A New Order [23/N]
#143298
Conversation
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
| //! This test checks that expressions like `0 << b` don't accidentally | ||
| //! modify the variable `b` due to codegen issues with virtual registers. | ||
| //! | ||
| //! Regression test for <https://github.com/rust-lang/rust/issues/152>. |
There was a problem hiding this comment.
Wow, old issue! That's only two months after https://www.github.com/rust-lang/rust/issues/1
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
`tests/ui`: A New Order [23/N] > [!NOTE] > > Intermediate commits are intended to help review, but will be squashed prior to merge. Some `tests/ui/` housekeeping, to trim down number of tests directly under `tests/ui/`. Part of rust-lang#133895. r? `@tgross35`
Rollup of 9 pull requests Successful merges: - #141532 (std: sys: net: uefi: tcp4: Implement write) - #143085 (Port `#[non_exhaustive]` to the new attribute parsing infrastructure) - #143298 (`tests/ui`: A New Order [23/N]) - #143372 (Remove names_imported_by_glob_use query.) - #143386 (Assign dependency bump PRs to me) - #143406 (Remove some unnecessary `unsafe` in VecCache) - #143408 (mbe: Gracefully handle macro rules that end after `=>`) - #143414 (remove special-casing of boxes from match exhaustiveness/usefulness analysis) - #143444 (clean up GVN TypeId test) r? `@ghost` `@rustbot` modify labels: rollup
bors
added
S-waiting-on-author
| } | ||
| assert_eq!(8, b); |
There was a problem hiding this comment.
Check size_of::<usize>() rather than a constant, this failed on 32bit.
Also you can update the above std::mem::size_of -> size_of since it's now in the prelude.
There was a problem hiding this comment.
uhm, do you mean like this?
pub fn main() {
let mut b: usize = 1;
while b < size_of::<usize>() {
// This shift operation should not mutate `b`
let _ = 0_usize << b;
b <<= 1;
}
assert_eq!(size_of::<usize>(), b);
}
There was a problem hiding this comment.
You can test 32-bit behavior on the playground by replacing usize with u32 :)
There was a problem hiding this comment.
yeah i know that usize's size is different on different platforms, but I wonder about test above, is that correct now? the reason im asking and not just run ./x test is because it will took 10 minutes and all resoures of my machince which im not ready to give it rn
There was a problem hiding this comment.
All I was planning to do to validate is put it in the playground anyway, it indeed looks correct https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=3aaf2f28b9cb4e4e60d14ae7624df83c
Noticing one other thing though; I think it might important that we observe the intermediate value so it can't get optimized out. println was doing this before but got deleted here, could you add std::hint::black_box(b); at the end of the while loop?
There was a problem hiding this comment.
i dont think that compiler can optimize this loop out, also i could miss understood you, i've added black_box to end of each iteration, and not after the while itself, not sure which one you exactly meant, but both should work fine i guess
|
hm, could this test be platform dependent? im not sure exactly what could cause this but when i was write this assert ive checked it and it was worked fine (its still do work fine in playground tho and CI passed, this is weird) Ah, ok, now i see the answer above |
|
Might as well |
`tests/ui`: A New Order [23/N]
<!-- homu-ignore:start -->
<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.
This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using
r? <reviewer name>
-->
<!-- homu-ignore:end -->
> [!NOTE]
>
> Intermediate commits are intended to help review, but will be squashed prior to merge.
Some `tests/ui/` housekeeping, to trim down number of tests directly under `tests/ui/`. Part of #133895.
r? `@tgross35`
try-job: dist-i586-gnu-i586-i686-musl
|
not sure if this needed becuase we made it dependable and not hardcoded 8, but let's see also not sure if i should force push tidy fix now or later, because im not sure what will happen with bors try |
|
fixed tidy, maybe need bors try once again |
|
yeah, safe to r+ |
Note
Intermediate commits are intended to help review, but will be squashed prior to merge.
Some
tests/ui/housekeeping, to trim down number of tests directly undertests/ui/. Part of #133895.r? @tgross35