-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Add debug assertions to write_bytes and copy* #62103
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
Seems reasonable to me! r=me with @Centril's nits |
@alexcrichton I had to disable some codegen tests in debug mode, because even with optimizations the new debug assertions get in the way. |
So the nits are fixed, but I'd like to get explicit confirmation that it is okay to ignore some (more) codegen tests in debug mode. We do run some CI runners in release mode, right? |
@bors: r+ |
📌 Commit d3e1bf9 has been approved by |
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Building libcore with debug assertions works just fine though locally for this PR. |
Line 601 in b820c76
This together with bjron3 comment should explain it. |
I think adding a second |
I see, that makes sense.
I am afraid I have no idea how linkers are invoked in general and in Rust specifically. My current inclination is to add some |
@RalfJung Replacing |
So is there some kind of guarantee that the arguments of |
@RalfJung Yes, that's exactly the difference between |
I would personally prefer to avoid updating any linkage with compiler-builtins, in general compiler-builtins should never be referencing panicking symbols. I agree with @nikic that the best solution for this is probably source-level changes in the compiler-builtins crate to avoid these assertions getting embedded, even in release mode. |
All right, submitted PR for compiler-builtins: rust-lang/compiler-builtins#300. |
I rebased the PR and added a commit that bumps compiler_builtins to the fresh 0.1.17 release. |
@bors: r+ |
📌 Commit 85d76a1 has been approved by |
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Rollup of 14 pull requests Successful merges: - #62103 (Add debug assertions to write_bytes and copy*) - #62405 (Remove never_type feature requirement for exhaustive patterns) - #62491 (Fix Pin urls in Option documentation) - #62533 (Add missing links for CannotReallocInPlace type) - #62634 (Less unsafe in the array example of MaybeUninit docs) - #62639 (Make VaListImpl<'f> invariant over the 'f lifetime) - #62646 (Tweak wording in feature gate errors) - #62662 (add spaces in front of trait requirements in libcore/cell.rs) - #62668 (Fix #62660) - #62673 (miri validation: better error messages for dangling references) - #62680 (Actually call `visit_block_entry` in `DataflowResultsConsumer`) - #62685 (Add info about undefined behavior to as_ref suggestions) - #62689 (Fix typo in RawWaker::new documentation) - #62698 (SGX target: don't pretend to be GNU/Linux to LLVM) Failed merges: r? @ghost
Looks like @nitnelave went MIA in #58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship.
Cc #53871