Output column number info when panicking #42938
Conversation
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
|
cc @rust-lang/libs About the addition of |
arielb1
added
the
S-waiting-on-author
|
I've renamed the commit (without any code changes) and added a second one with a small refactor, a comment for the stage0 updater, and (hopefully) a fix for the test failure. |
|
👍 |
est31
changed the title
|
ready. r? @eddyb |
aidanhs
added
S-waiting-on-review
There was a problem hiding this comment.
Ok looks pretty good to me, thanks @est31! Mind doing some size analysis to ensure this doesn't increase the size of anything too much?
src/libcore/panicking.rs
Outdated
| #[allow(improper_ctypes)] | ||
| extern { | ||
| #[lang = "panic_fmt"] | ||
| #[unwind] | ||
| fn panic_impl(fmt: fmt::Arguments, file: &'static str, line: u32) -> !; | ||
| fn panic_impl(fmt: fmt::Arguments, file: &'static str, line: u32, col :u32) -> !; |
I've compiled rustc with the patch and with the commit this PR bases on, and these are the results: Numbers come from du output, and are therefore in kb. I've rm -r'd both directories beforehand, so artifacts of other builds don't overlap. So almost no noticeable change, its in the sub-0.1% range for both std and rustc. |
|
@bors r+ |
|
📌 Commit 376c6e7 has been approved by |
|
⌛ Testing commit 376c6e71b0aa8bc62d5ff4a469d35a6feb063c0c with merge 018afe2f0682464086a7ebae11637bf160a4a711... |
|
💔 Test failed - status-travis |
|
The failure looks legit, I'll investigate. |
|
The error is inside cargo's testsuite: (expand) |
Let the cargo_bench_failing_test tolerate col info Needed by rust-lang/rust#42938
|
@Mark-Simulacrum are you sure that an update to Cargo.toml/Cargo.lock is needed? E.g. look at git commit 77931c2, it has no update. About the other cargo failures... I'll try to fix them as well. Right now my hardware is a bit limited so I can't really run the testsuite sadly... |
|
Cargo.lock was updated though: 77931c2#diff-5d9bf57935c52008319cccc88d4d737dR160. |
Needed by rust-lang/rust#42938 I've now ripgrepped for "panicked at" and found no further test that hardcodes the "filename:line$" format.
Let two further tests tolerate col info in panics Needed by rust-lang/rust#42938 I've now ripgrepped for "panicked at" and found no further test that hardcodes the "filename:line$" format.
... to get rust-lang/cargo#4244 and rust-lang/cargo#4246
|
@bors r+ |
|
📌 Commit 3b91f94 has been approved by |
Output column number info when panicking Outputs the column number when panicking. Useful when you e.g. have code like `foo[i] = bar[k] + bar[l]` and you get a panic with index out of bounds, or when you have an expression like `a = b + c + d + e` and the addition overflows. Now you know which operation to blame! The format is `file:line:column`, just like for compiler errors. Example output with the patch: ``` thread 'main' panicked at 'index out of bounds: the len is 5 but the index is 8', src/main.rs:3:8 ``` As some of the API between the compiler and the library landscape gets broken, this is a bit hackier than I'd originally wanted it to be. * `panic` and `panic_bounds_check` lang items got an additional column param, on stage0 I still have to use the previous version. After a SNAP this should be resolved. * For `#[derive(RustcDeserialze)]`, stage0 requires a fixed signature for `std::rt::begin_panic`, so we can't change it right away. What we need to do instead is to keep the signature, and add a `begin_panic_new` function that we use in later stages instead. After a SNAP we can change the `begin_panic` function and rely on it instead of `begin_panic_new`, and one SNAP later we can remove `begin_panic_new`. * Fortunately I didn't have to break anything about the panic hook API, I could easily extend it. Note that debuginfo remains unchanged, so RUST_BACKTRACE output won't contain any column info. See issue #42921 for discussion on including the column in debuginfo.
|
☀️ Test successful - status-appveyor, status-travis |
Needed by rust-lang/rust#42938 I've now ripgrepped for "panicked at" and found no further test that hardcodes the "filename:line$" format.
In rust-lang#42938 we made the compiler emit a call to begin_panic_new in order to pass column info to it. Now with stage0 updated (rust-lang#43320), we can safely change begin_panic and start emitting calls for it again.
I've added the panic_col feature in PR rust-lang#42938. Now it's time to stabilize it! Closes rust-lang#42939.
Stabilize the panic_col feature I've added the panic_col feature in PR rust-lang#42938. Now it's time to stabilize it! Closes rust-lang#42939.
Outputs the column number when panicking. Useful when you e.g. have code like
foo[i] = bar[k] + bar[l]and you get a panic with index out of bounds, or when you have an expression likea = b + c + d + eand the addition overflows. Now you know which operation to blame!The format is
file:line:column, just like for compiler errors. Example output with the patch:As some of the API between the compiler and the library landscape gets broken, this is a bit hackier than I'd originally wanted it to be.
panicandpanic_bounds_checklang items got an additional column param, on stage0 I still have to use the previous version. After a SNAP this should be resolved.#[derive(RustcDeserialze)], stage0 requires a fixed signature forstd::rt::begin_panic, so we can't change it right away. What we need to do instead is to keep the signature, and add abegin_panic_newfunction that we use in later stages instead. After a SNAP we can change thebegin_panicfunction and rely on it instead ofbegin_panic_new, and one SNAP later we can removebegin_panic_new.Note that debuginfo remains unchanged, so RUST_BACKTRACE output won't contain any column info. See issue #42921 for discussion on including the column in debuginfo.