tests/ui: A New Order [23/N]#143298
Conversation
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 |
`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
| let _ = 0_usize << b; | ||
| b <<= 1; | ||
| } | ||
| 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+ |
|
@bors r+
It's pretty common to run a try job when adding a fix for something that failed, even if it's reasonably trivial like this |
`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: - #141996 (Fix `proc_macro::Ident`'s handling of `$crate`) - #142911 (Remove support for dynamic allocas) - #142950 (mbe: Rework diagnostics for metavariable expressions) - #143270 (tests/codegen/enum/enum-match.rs: accept negative range attribute) - #143298 (`tests/ui`: A New Order [23/N]) - #143398 (tidy: add support for `--extra-checks=auto:` feature) - #143632 (fix: correct parameter names in LLVMRustBuildMinNum and LLVMRustBuildMaxNum FFI declarations) - #143644 (Add triagebot stdarch mention ping) - #143651 (Win: Use exceptions with empty data for SEH panic exception copies instead of a new panic) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 9 pull requests Successful merges: - #141996 (Fix `proc_macro::Ident`'s handling of `$crate`) - #142950 (mbe: Rework diagnostics for metavariable expressions) - #143011 (Make lint `ambiguous_glob_imports` deny-by-default and report-in-deps) - #143265 (Mention as_chunks in the docs for chunks) - #143270 (tests/codegen/enum/enum-match.rs: accept negative range attribute) - #143298 (`tests/ui`: A New Order [23/N]) - #143396 (Move NaN tests to floats/mod.rs) - #143398 (tidy: add support for `--extra-checks=auto:` feature) - #143644 (Add triagebot stdarch mention ping) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #143298 - Kivooeo:tf23, r=tgross35 `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 #133895. r? ``@tgross35``
Rollup of 9 pull requests Successful merges: - rust-lang#141996 (Fix `proc_macro::Ident`'s handling of `$crate`) - rust-lang#142950 (mbe: Rework diagnostics for metavariable expressions) - rust-lang#143011 (Make lint `ambiguous_glob_imports` deny-by-default and report-in-deps) - rust-lang#143265 (Mention as_chunks in the docs for chunks) - rust-lang#143270 (tests/codegen/enum/enum-match.rs: accept negative range attribute) - rust-lang#143298 (`tests/ui`: A New Order [23/N]) - rust-lang#143396 (Move NaN tests to floats/mod.rs) - rust-lang#143398 (tidy: add support for `--extra-checks=auto:` feature) - rust-lang#143644 (Add triagebot stdarch mention ping) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 9 pull requests Successful merges: - rust-lang/rust#141996 (Fix `proc_macro::Ident`'s handling of `$crate`) - rust-lang/rust#142950 (mbe: Rework diagnostics for metavariable expressions) - rust-lang/rust#143011 (Make lint `ambiguous_glob_imports` deny-by-default and report-in-deps) - rust-lang/rust#143265 (Mention as_chunks in the docs for chunks) - rust-lang/rust#143270 (tests/codegen/enum/enum-match.rs: accept negative range attribute) - rust-lang/rust#143298 (`tests/ui`: A New Order [23/N]) - rust-lang/rust#143396 (Move NaN tests to floats/mod.rs) - rust-lang/rust#143398 (tidy: add support for `--extra-checks=auto:` feature) - rust-lang/rust#143644 (Add triagebot stdarch mention ping) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 9 pull requests Successful merges: - rust-lang/rust#141996 (Fix `proc_macro::Ident`'s handling of `$crate`) - rust-lang/rust#142950 (mbe: Rework diagnostics for metavariable expressions) - rust-lang/rust#143011 (Make lint `ambiguous_glob_imports` deny-by-default and report-in-deps) - rust-lang/rust#143265 (Mention as_chunks in the docs for chunks) - rust-lang/rust#143270 (tests/codegen/enum/enum-match.rs: accept negative range attribute) - rust-lang/rust#143298 (`tests/ui`: A New Order [23/N]) - rust-lang/rust#143396 (Move NaN tests to floats/mod.rs) - rust-lang/rust#143398 (tidy: add support for `--extra-checks=auto:` feature) - rust-lang/rust#143644 (Add triagebot stdarch mention ping) r? `@ghost` `@rustbot` modify labels: rollup
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