Skip to content
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

Test gdb pretty printing more and fix overzealous type substitution #68098

Merged
merged 1 commit into from
Mar 22, 2020

Conversation

ssomers
Copy link
Contributor

@ssomers ssomers commented Jan 10, 2020

Adresses a problem concerning printing BTreeMap / BTreeSet data in gdb: when the key or value type name contains substring "LeafNode", and the map has multiple nodes (e.g. more than 11 elements), printing causes an exception. E.g.

rustc -g - <<EOF
    use std::collections::BTreeMap;

    struct MyLeafNode(i8);

    fn main() {
        let m: BTreeMap<i8, MyLeafNode> = (0..12).map(|i| (i, MyLeafNode(i))).collect();
        assert!(!m.is_empty());
    }
EOF
$ rust-gdb rust_out
(gdb) b 7
(gdb) r
(gdb) p m
$1 = BTreeMap<i8, rust_out::MyLeafNode>(len: 12)Python Exception <class 'gdb.error'> No type named alloc::collections::btree::node::InternalNode<i8, rust_out::MyInternalNode>.:
use std::collections::BTreeMap;

The code was written in #56144 by @tromey (and later touched upon by @RalfJung in #57045, but I think that had nothing to do with the issues in this PR).

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 10, 2020
@Mark-Simulacrum
Copy link
Member

I think someone recently looked into our gdb pretty printing -- maybe @pnkfelix? Or @michaelwoerister?

@Dylan-DPC-zz
Copy link

r? @pnkfelix

@tromey
Copy link
Contributor

tromey commented Feb 1, 2020

FWIW this patch looks reasonable to me.

@ssomers
Copy link
Contributor Author

ssomers commented Feb 2, 2020

Last push is a (very late) rebase that I had already prepared for #67980

@pnkfelix
Copy link
Member

(This will need to be rebased atop PR #60826 after that lands.)

@pnkfelix
Copy link
Member

r=me after PR #60826 lands and you do subsequent rebase.

@JohnTitor JohnTitor added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 15, 2020
@ssomers
Copy link
Contributor Author

ssomers commented Mar 19, 2020

Last commit removed the part that isn't in the title and would interfere with #70111.

@tromey
Copy link
Contributor

tromey commented Mar 20, 2020

Thank you for doing this. FWIW this still looks reasonable to me.

@Mark-Simulacrum
Copy link
Member

r=me but would like to wait on #70111 landing I think

@bors
Copy link
Contributor

bors commented Mar 21, 2020

☔ The latest upstream changes (presumably #70205) made this pull request unmergeable. Please resolve the merge conflicts.

@ssomers
Copy link
Contributor Author

ssomers commented Mar 21, 2020

I noticed that if the key type is (), and you insert an element (the only one ever), pretty printing says the type is undefined. I guess civilization can survive without me logging that as a bug.

r? @msimulacrum

@Mark-Simulacrum
Copy link
Member

It looks like #60826 is still a bit stalled and should be fairly easy to rebase atop this given the minimal nature of this patch. @bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Mar 21, 2020

📌 Commit d8a136f has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Mar 21, 2020
@bors
Copy link
Contributor

bors commented Mar 22, 2020

⌛ Testing commit d8a136f with merge 94d43d6...

@bors
Copy link
Contributor

bors commented Mar 22, 2020

☀️ Test successful - checks-azure
Approved by: Mark-Simulacrum
Pushing 94d43d6 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 22, 2020
@bors bors merged commit 94d43d6 into rust-lang:master Mar 22, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 26, 2020
…r=Mark-Simulacrum

Test and fix gdb pretty printing more

Over time I had oversimplified the test case for rust-lang#68098: it does not have an internal node to print so it does not test what it pretend to test. And then I also realized not spotting the same mistake reviewing rust-lang#70111, and more likely to occur in the wild. Now, both test cases fail if you put back the flawed python code.

r? @Mark-Simulacrum
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 27, 2020
…Mark-Simulacrum

Test and fix gdb pretty printing more

Over time I had oversimplified the test case for rust-lang#68098: it does not have an internal node to print so it did not test what it pretended to test. And then I also realized not spotting the same mistake reviewing rust-lang#70111, and more likely to occur in the wild. Now, both test cases fail if you put back the flawed python code.

r? @Mark-Simulacrum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants