Skip to content

Use lazy iterator in vec/slice gdb pretty printers #34639

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

Merged
merged 1 commit into from
Jul 5, 2016
Merged

Use lazy iterator in vec/slice gdb pretty printers #34639

merged 1 commit into from
Jul 5, 2016

Conversation

dzamlo
Copy link
Contributor

@dzamlo dzamlo commented Jul 4, 2016

No description provided.

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

// except according to those terms.

// ignore-windows failing on win32 bot
// ignore-freebsd: gdb package too new
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on what that means?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the same ignore-* lines as the pretty-std.rs test, because the tests I added test a similar part of the code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's OK then.

@michaelwoerister
Copy link
Member

Thanks a lot for the PR! If you take care of the nits I pointed out, I'd be happy to have it merged.

@dzamlo
Copy link
Contributor Author

dzamlo commented Jul 4, 2016

Should I make a second commit with the changes or do you prefer that I squash the requested changes with the existing commit?

@srinivasreddy
Copy link
Contributor

I would generally, squash them. But either of it is okay, i guess.

@michaelwoerister
Copy link
Member

Should I make a second commit with the changes or do you prefer that I squash the requested changes with the existing commit?

Please just squash them or use git commit --amend.

@dzamlo
Copy link
Contributor Author

dzamlo commented Jul 4, 2016

I think the nits have been taken care of.

@michaelwoerister
Copy link
Member

Excellent! Thanks a lot!

@michaelwoerister
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 4, 2016

📌 Commit 5de76af has been approved by michaelwoerister

@bors
Copy link
Collaborator

bors commented Jul 4, 2016

⌛ Testing commit 5de76af with merge d7ef1ac...

bors added a commit that referenced this pull request Jul 4, 2016
Use lazy iterator in vec/slice gdb pretty printers
@bors bors merged commit 5de76af into rust-lang:master Jul 5, 2016
@alexcrichton
Copy link
Member

Hm so on the Linux bots (where these tests passed) the GDB we're running is:

GNU gdb (Ubuntu 7.7.1-0ubuntu5~14.04.2) 7.7.1

whereas on the nightly bots (slightly different due to build funkiness) the GDB is custom-built and is:

GNU gdb (GDB) 7.11

Unfortunately the test added here is failing on the nightly bot (full logs):


failures:

---- [debuginfo-gdb] debuginfo-gdb/pretty-huge-vec.rs stdout ----
    NOTE: compiletest thinks it is using GDB version 7.11

error: line not found in debugger output: $1 = Vec<u8>(len: 1000000000, cap: 1000000000) = {[...]...}
status: exit code: 0
command: gdb -quiet -batch -nx -command=x86_64-unknown-linux-gnu/test/debuginfo-gdb/pretty-huge-vec.debugger.script
stdout:
------------------------------------------
GNU gdb (GDB) 7.11
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word".
Breakpoint 1 at 0xcac: file /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/tmp/distcheck/rustc-nightly/src/test/debuginfo/pretty-huge-vec.rs, line 38.
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Breakpoint 1, pretty_huge_vec::main () at /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/tmp/distcheck/rustc-nightly/src/test/debuginfo/pretty-huge-vec.rs:38
38      zzz(); // #break
$1 = Vec<u8>(len: 1000000000, cap: 1000000000)
$2 = &[u8](len: 1000000000)
A debugging session is active.

    Inferior 1 [process 25786] will be killed.

Quit anyway? (y or n) [answered Y; input not from terminal]

------------------------------------------
stderr:
------------------------------------------
Python Exception <type 'exceptions.MemoryError'> : 
Python Exception <type 'exceptions.MemoryError'> : 

------------------------------------------

thread '[debuginfo-gdb] debuginfo-gdb/pretty-huge-vec.rs' panicked at 'explicit panic', /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/tmp/distcheck/rustc-nightly/src/tools/compiletest/src/runtest.rs:2243
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    [debuginfo-gdb] debuginfo-gdb/pretty-huge-vec.rs

test result: FAILED. 100 passed; 1 failed; 3 ignored; 0 measured

Maybe this fix only applies to older GDB versions? Maybe we need to recompile gdb somewhat differently than what we do today?

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jul 5, 2016
This ignores a test added in rust-lang#34639 because unfortunately the nightly bots are
now broken, but this is now tracked in rust-lang#34662.
bors added a commit that referenced this pull request Jul 5, 2016
test: Ignore pretty-huge-vec.rs to fix nightlies

This ignores a test added in #34639 because unfortunately the nightly bots are
now broken, but this is now tracked in #34662.
@bruno-medeiros
Copy link

Sweet. Which Rust release version can we expect this fix to go into?

@steveklabnik
Copy link
Member

@bruno-medeiros it landed just before the branch to beta, so I believe this will land in Rust 1.11, six weeks from today.

@bruno-medeiros
Copy link

@bruno-medeiros it landed just before the branch to beta, so I believe this will land in Rust 1.11, six weeks from today.

And will this need an upcoming GDB version, or will it work in existing GDB versions?

@michaelwoerister
Copy link
Member

@bruno-medeiros

And will this need an upcoming GDB version, or will it work in existing GDB versions?

I think this should work with quite old versions of GDB.

@alexcrichton I think we should pass --with-python to ./configure.

@alexcrichton
Copy link
Member

@michaelwoerister uh, sure! Remind me again which bot should pass which value though?

@michaelwoerister
Copy link
Member

@alexcrichton
Copy link
Member

Oh right, I see what you mean! I'll give that a spin and see if it works.

@bruno-medeiros
Copy link

@michaelwoerister So this new thing: https://www.phoronix.com/scan.php?page=news_item&px=GNU-Toolchain-Q2-2016 is unrelated to this issue?

@michaelwoerister
Copy link
Member

Yes, this is just an improvement to the Python-based pretty printers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants