Skip to content

Conversation

gentoo90
Copy link
Contributor

Pretty-printing of Option failed with exception:

Traceback (most recent call last):
  File "/opt/rust-bin-9999/lib/rustlib/etc/gdb_rust_pretty_printing.py", line 166, in rust_pretty_printer_lookup_function
    if encoded_enum_info.is_null_variant():
  File "/opt/rust-bin-9999/lib/rustlib/etc/debugger_pretty_printers_common.py", line 295, in is_null_variant
    return discriminant_val.as_integer() == 0
  File "/opt/rust-bin-9999/lib/rustlib/etc/gdb_rust_pretty_printing.py", line 83, in as_integer
    return int(self.gdb_val)
gdb.error: Cannot convert value to long.
Before After
gdb_before gdb_after

main.rs

fn main() {
    let vec_u8: Option<Vec<u8>> = Some(vec![1,2,3]);
    let empty_vec_u8: Option<Vec<u8>> = None;
    let string: Option<String> = Some("I'm an optional string!".to_owned());
    let empty_string: Option<String> = None;

     println!("breakpoint here");
}

@rust-highfive
Copy link
Contributor

r? @aturon

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

@gentoo90
Copy link
Contributor Author

r? @michaelwoerister

@michaelwoerister
Copy link
Member

Thanks, @gentoo90! Hm, this seem rather like an accidental fix of the issue. A fat pointer should not have any struct fields that we can further recurse into.

@eddyb, do optimized enums works with byte offsets for the discriminant now?

@eddyb
Copy link
Member

eddyb commented Jan 22, 2018

A fat pointer should not have any struct fields that we can further recurse into.

Yes it does (at least internally). However, we should be encoding a byte offset and some other information (not currently done). @tromey knows more about the ideal solution here.

@michaelwoerister
Copy link
Member

What I meant was: A fat pointer should not have any fields that are themselves structs.

@eddyb
Copy link
Member

eddyb commented Jan 22, 2018

@michaelwoerister Oh huh, this implementation doesn't even read the encoded field path?
Anyway, the issue I was thinking of is #46173, with the latest updates at #32920 (comment).

@michaelwoerister
Copy link
Member

The field path should be in __disr_field_indices if I read this correctly.
The python-based pretty printers should also be able to just read bytes from memory and convert them. It would have to know where to read and how much to read though -- which could be encoded in the field name instead of the field path. The proper implementation that @tromey is working on will probably still take some time.

@eddyb
Copy link
Member

eddyb commented Jan 22, 2018

@michaelwoerister I assumed that the decoder would be in gdb / lldb themselves, not some in-tree python scripts, otherwise I would've changed it back in #45225.

@michaelwoerister
Copy link
Member

For now, we still rely on the in-tree pretty printers mostly, I think.

@michaelwoerister
Copy link
Member

Or at least, they are still used often, so we should keep them working.

@carols10cents carols10cents added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 22, 2018
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 31, 2018
@kennytm
Copy link
Member

kennytm commented Jan 31, 2018

Hi @gentoo90! Just to check if you are still on this.

@michaelwoerister, I've change the tag back to S-waiting-on-review. Could you clarify what OP should do to the PR? If I understand #47617 (comment) correctly, what you mean is the fix is not correct (String and Vec<u8> are not fat pointers for instance), and the original code comment about fat pointer is invalid either?

@pietroalbini
Copy link
Member

@michaelwoerister ping from triage!

@shepmaster
Copy link
Member

/cc @rust-lang/compiler — we haven't heard from @michaelwoerister in a week or two; is anyone else able to review this?

@michaelwoerister
Copy link
Member

Back from vacation now...

I'm going to close this since it only fixes the problem accidentally and would probably fail for more complicated cases than Option.

@gentoo90 gentoo90 deleted the gdb-pretty-printers branch February 12, 2018 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants