make ty::Ty: Debug not call the Display impl
#107084
Closed
+1,401
−332
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
|
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri |
|
I'm not sure that the change to every single mir-opt .mir file is reasonable lol |
This comment has been minimized.
This comment has been minimized.
BoxyUwU
force-pushed
the
ty_debug_actually_debug
branch
2 times, most recently
from
f6cc567
to
920ce60
Compare
This comment has been minimized.
This comment has been minimized.
BoxyUwU
force-pushed
the
ty_debug_actually_debug
branch
from
920ce60
to
0ba83f3
Compare
This comment has been minimized.
This comment has been minimized.
BoxyUwU
force-pushed
the
ty_debug_actually_debug
branch
from
0ba83f3
to
b9b02ae
Compare
This comment has been minimized.
This comment has been minimized.
BoxyUwU
force-pushed
the
ty_debug_actually_debug
branch
from
b9b02ae
to
778616a
Compare
compiler-errors
approved these changes
Jan 20, 2023
There was a problem hiding this comment.
Aside from one nit in the graphviz printing code, this looks good to me! 🦀
I'm alright with all the UI test changes that have to do with rustc_* testing attributes. The other UI test suite changes seem to do with no longer trimming paths due to using {} over {:?}.
Mir-opt test suite changes seem to do with the fact we debug-print consts, but I don't think that needs fixing.
I'm pretty convinced this does not needs an MCP or anything, since it's predominantly internal-facing and we don't guarantee anything about the stability of the places (e.g. MIR) that this PR pokes through.
I'll wait a few days to approve this just so nightly can branch (in the off chance that changing the path trimming behavior of something might cause an ICE, though really I doubt it) and so that anyone else can bring up comments that they have.
|
Does this affect debug printing of The compiler has a bunch of types without |
RalfJung
reviewed
Jan 20, 2023
|
as far as I can tell |
|
Ah right, I slightly misremembered. There is the TyAndLayout type that is used quite a bit in the interpreter (and codegen I think?).
|
|
Specifically I am referring to this type whose |
|
@bors r+ |
|
📌 Commit 868dfc698c22638b081b7b5da12f03dff376d90f has been approved by It is now in the queue for this repository. |
bors
added
S-waiting-on-bors
RalfJung
reviewed
Jan 27, 2023
| impl<'a, Ty: fmt::Debug + fmt::Display> fmt::Debug for ArgAbi<'a, Ty> { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| f.debug_struct("ArgAbi").field("layout", &self.layout).field("mode", &self.mode).finish() | ||
| } |
There was a problem hiding this comment.
Why does this become necessary?
EDIT: Oh I see, it's the new trait bounds on the TyAndLayout debug impl. That's unfortunate... but still, this seems like the best option then.
|
☔ The latest upstream changes (presumably #101692) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
⌛ Testing commit 31b3373 with merge 084f72ec2807917a72f0c74449505484af9a2ea3... |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message |
1 similar comment
|
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message |
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message |
|
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message |
|
@BoxyUwU any updates on this? |
|
no I still just need to rebase this and fix the CI failures |
|
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
May 29, 2023
…compiler-errors Make `TyKind: Debug` have less verbose output Current `TyKind: Debug` impl is basically unusable for debugging, its too verbose even for verbose debugging 🤣 This PR replaces the debug logic for `TyKind` with a more manual debug impl instead of a hand expanded derived impl. This should help make rust-lang#107084 more reasonable to land since the output of `Ty: Debug` will be better. This isn't a fully completed change to the `Debug` impl of `TyKind` as there's still logic from the derive macro for some variants. Some of the variants are also not consisten with the `-Zverbose` printing of `Ty`, ideally `-Zverbose` printing of `Ty` would also just defer to the debug impl instead of having lots of checks in pretty printing. I plan on fixing this in follow up PRs since it seems tricky to do in this one and its already a large PR 😅
|
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message |
|
@BoxyUwU what's the status of this? |
|
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message |
Dylan-DPC
removed
the
S-waiting-on-author
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Sep 16, 2023
…er-errors make interpreter and TyAndLayout type Debug impl independent of Ty debug impl This fixes some (but not all) of the fallout from rust-lang#115661. Second commit is taken from rust-lang#107084 (and slightly adjusted); I preserved the original git author information.
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 16, 2023
Rollup merge of rust-lang#115866 - RalfJung:interpret-debug, r=compiler-errors make interpreter and TyAndLayout type Debug impl independent of Ty debug impl This fixes some (but not all) of the fallout from rust-lang#115661. Second commit is taken from rust-lang#107084 (and slightly adjusted); I preserved the original git author information.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ty'sDebugimpl currently callsTy'sDisplayimpl. This means that ICE messages, debug statements etc in rustc all print out things that are missing useful details.r? @compiler-errors