Improve CTFE UB validation error messages #86275
Conversation
| @@ -26,23 +26,27 @@ use super::{ | |||
|
|
|||
| macro_rules! throw_validation_failure { | |||
| ($where:expr, { $( $what_fmt:expr ),+ } $( expected { $( $expected_fmt:expr ),+ } )?) => {{ | |||
| let msg = rustc_middle::ty::print::with_no_trimmed_paths(|| { | |||
| let (path, msg) = rustc_middle::ty::print::with_no_trimmed_paths(|| { | |||
There was a problem hiding this comment.
Why a single big let for both of them, and not two separate lets?
There was a problem hiding this comment.
it was just not to have two separate rustc_middle::ty::print::with_no_trimmed_path calls: I'm unsure whether the non-path expressions actually required being wrapped by it when they are serialized into strings (and was unsure of the costs), and so found it safer to not change this macro too much.
I can try changing this to have 2 lets but a single with_no_trimmed_path() call for the actual path, and see whether it changes the test output. Or have 2 lets and 2 calls. Let me know what you prefer and I'll do it.
There was a problem hiding this comment.
The msg part should not need that with_no_trimmed_paths thing, so maybe try two lets with just the path part having with_no_trimmed_paths and then the test suite will tell us if that works or not.
There was a problem hiding this comment.
alright, I'll try that tonight, thanks for the review !
There was a problem hiding this comment.
The ui test suite seems to agree that the msg doesn't need anything related to path trimming, so I've pushed a separate commit (to help with review) to address this point.
|
Thanks, this looks great. :) |
|
✌️ @lqd can now approve this pull request |
|
thanks I already have bors rights ^^ I'll monitor CI and get this landed :) |
Nice. :) I cannot remember who does so I usually err on the side of delegating. ;) |
|
@bors r=RalfJung |
|
📌 Commit 5af1c72 has been approved by |
bors
added
the
S-waiting-on-bors
Improve CTFE UB validation error messages As mentioned in rust-lang#86245 (comment) this PR slightly improves the formatting of validation errors, to move the path to the error prefix. From: `type validation failed: encountered invalid vtable: size is bigger than largest supported object at .0` To: `type validation failed at .0: encountered invalid vtable: size is bigger than largest supported object`.
|
☀️ Test successful - checks-actions |
Tested on commit rust-lang/rust@539d7bd. Direct link to PR: <rust-lang/rust#86275> 💔 miri on windows: test-pass → test-fail (cc @eddyb @RalfJung @oli-obk). 💔 miri on linux: test-pass → test-fail (cc @eddyb @RalfJung @oli-obk).
As mentioned in #86245 (comment) this PR slightly improves the formatting of validation errors, to move the path to the error prefix.
From:
type validation failed: encountered invalid vtable: size is bigger than largest supported object at .0To:
type validation failed at .0: encountered invalid vtable: size is bigger than largest supported object.