-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Remove the call that makes miri fail #70038
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
@oli-obk we usually test these things in ui tests, right? Would you prefer this to become a ui test instead? |
…xplain it cant be called in ctfe yet
Yea a ui test would be more in line with our other tests |
Okay, so @DutchGhost can you make this into a ui test? You can find existing tests in https://github.com/rust-lang/rust/tree/master/src/test/ui/consts. |
r? @RalfJung |
src/test/ui/consts/const_forget.rs
Outdated
@@ -0,0 +1,22 @@ | |||
// run-pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a const-test with empty main
, why is check-pass
not sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DutchGhost this blocks enabling leak checks again in https://github.com/RalfJung/miri-test-libstd, would be good if we could land this quickly. :)
Thanks, that was quick. :) |
📌 Commit d6f3a43 has been approved by |
…lfJung Remove the call that makes miri fail Fixes the concern raised in https://github.com/rust-lang/rust/pull/69645/files#r392884274 cc @RalfJung
…lfJung Remove the call that makes miri fail Fixes the concern raised in https://github.com/rust-lang/rust/pull/69645/files#r392884274 cc @RalfJung
…lfJung Remove the call that makes miri fail Fixes the concern raised in https://github.com/rust-lang/rust/pull/69645/files#r392884274 cc @RalfJung
Rollup of 16 pull requests Successful merges: - rust-lang#65097 (Make std::sync::Arc compatible with ThreadSanitizer) - rust-lang#69033 (Use generator resume arguments in the async/await lowering) - rust-lang#69997 (add `Option::{zip,zip_with}` methods under "option_zip" gate) - rust-lang#70038 (Remove the call that makes miri fail) - rust-lang#70058 (can_begin_literal_maybe_minus: `true` on `"-"? lit` NTs.) - rust-lang#70111 (BTreeMap: remove shared root) - rust-lang#70139 (add delay_span_bug to TransmuteSizeDiff, just to be sure) - rust-lang#70165 (Remove the erase regions MIR transform) - rust-lang#70166 (Derive PartialEq, Eq and Hash for RangeInclusive) - rust-lang#70176 (Add tests for rust-lang#58319 and rust-lang#65131) - rust-lang#70177 (Fix oudated comment for NamedRegionMap) - rust-lang#70184 (expand_include: set `.directory` to dir of included file.) - rust-lang#70187 (more clippy fixes) - rust-lang#70188 (Clean up E0439 explanation) - rust-lang#70189 (Abi::is_signed: assert that we are a Scalar) - rust-lang#70194 (#[must_use] on split_off()) Failed merges: r? @ghost
Fixes the concern raised in https://github.com/rust-lang/rust/pull/69645/files#r392884274
cc @RalfJung