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

tracking issue for debug_non_exhaustive feature #67364

Closed
5 tasks done
richard-uk1 opened this issue Dec 17, 2019 · 26 comments
Closed
5 tasks done

tracking issue for debug_non_exhaustive feature #67364

richard-uk1 opened this issue Dec 17, 2019 · 26 comments
Labels
A-fmt Area: `std::fmt` B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@richard-uk1
Copy link
Contributor

richard-uk1 commented Dec 17, 2019

Feature gate: #![feature(debug_non_exhaustive)]

This is a tracking issue for adding the ability to add an ellipsis to the end of the Debug representation of a struct, to indicate that the struct has more fields, but that these fields are not displayable. This involves adding the finish_non_exhaustive method to DebugStruct, that produces output like

Name { field1: value1, .. }

where the .. indicate that there are more hidden fields.

Public API

// core::fmt

impl<'a, 'b: 'a> DebugStruct<'a, 'b> {
    pub fn finish_non_exhaustive(&mut self) -> fmt::Result;
}

Steps / History

Unresolved Questions

  • Should this method be implemented for DebugTuple and DebugMap as well?
    • I would probably say "no" since I can't think of any use cases for them. They could always be added later.
@JohnTitor JohnTitor added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Dec 17, 2019
@JohnTitor JohnTitor added B-unstable Blocker: Implemented in the nightly compiler and unstable. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Feb 28, 2020
@rossmacarthur
Copy link
Contributor

rossmacarthur commented Jun 21, 2020

Any ETA as to when this might be stabilized? What is blocking this being stabilized?

@richard-uk1
Copy link
Contributor Author

AFAIK there aren't any blockers to stabilizing. Things normally live in nightly so they can be tested, but in this case it's unlikely someone will drop stable support just so they can use this feature. I think it will only be used once it's stable.

@richard-uk1
Copy link
Contributor Author

richard-uk1 commented Jun 21, 2020

It's a small low-risk change (since the methods can be reimplemented or deprecated if they are really broken).

@rossmacarthur
Copy link
Contributor

Although not its primary intention I also find this feature quite useful when implementing Debug for structs that can't be derived. Where you still want to indicate that not all fields are shown. For example:

struct Example {
    field: String,
    func: Box<dyn Fn(&str) -> bool + 'static>
}

impl fmt::Debug for Example {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        f.debug_struct("Example")
            .field("field", &self.field)
            .finish_non_exhaustive()
    }
}

@richard-uk1
Copy link
Contributor Author

It could be possible to extend #[derive(Debug)] to work on structs/enums with non-Derive fields, by skipping them and marking it non-exhaustive.

@kornelski
Copy link
Contributor

I also have a use-case for partially printing structs. In my case printing all of struct's fields in a meaningful way would be rather complex and require potentially-expensive FFI calls, so I'd rather omit them from debug output, but I still want to leave a hint that they exist.

@bbqsrc
Copy link

bbqsrc commented Jul 12, 2020

I have a use case for printing expensive structs as well.

@KodrAus KodrAus added I-nominated A-fmt Area: `std::fmt` Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. labels Jul 29, 2020
@dimpolo
Copy link
Contributor

dimpolo commented Nov 4, 2020

It would be nice to have a finish_non_exhaustive method on DebugTuple for my crate.

@richard-uk1
Copy link
Contributor Author

It would be nice to have a finish_non_exhaustive method on DebugTuple for my crate.

Maybe its time to revisit the open question. I could implement this if there were consensus it's a good idea.

@m-ou-se
Copy link
Member

m-ou-se commented Dec 8, 2020

Any ETA as to when this might be stabilized? What is blocking this being stabilized?

Looks like there are no blockers. There's still the question of whether this is also wanted on DebugList, DebugMap, DebugSet, and/or DebugTuple. But that doesn't have to block stabilization of DebugStruct::finish_non_exhaustive.

@guswynn
Copy link
Contributor

guswynn commented Mar 11, 2021

I think that at least for DebugTuple finish_non_exhaustive makes sense as its at least partially is intended to be used with tuple structs. As for DebugList, DebugMap, and DebugSet, I would guess that people wanting this would be less common, but it is likely a small enough api surface to be uncontroversial. @derekdreery For me, DebugStruct::finish_non_exhaustive is nice enough that I will likely submit a stabilization PR, and if you are no longer interested in creating a new feature for impls on the other ones, I may be able to find time to do that as well!

@richard-uk1
Copy link
Contributor Author

Hey @guswynn can you put a note on here if you start working on adding support for the other struct types, to make sure that we don't duplicate work. :)

@m-ou-se
Copy link
Member

m-ou-se commented Mar 13, 2021

@rfcbot merge

DebugStruct::finish_non_exhaustive() is an alternative to DebugStruct::finish() that prints , .. } instead of }.

Similar functionality on DebugList, DebugMap, DebugSet, and/or DebugTuple was briefly discussed but seems less important, so is not included here.

@rfcbot
Copy link

rfcbot commented Mar 13, 2021

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Mar 13, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Mar 13, 2021

I've added a to do item to the steps above:

  • Replace .finish() by .finish_non_exhaustive() within the standard library where it makes sense.

Not critical, but it'd be nice if we (std/core) set the right example.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Mar 13, 2021
@rfcbot
Copy link

rfcbot commented Mar 13, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Mar 23, 2021
@rfcbot
Copy link

rfcbot commented Mar 23, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Mar 23, 2021
@guswynn
Copy link
Contributor

guswynn commented Mar 24, 2021

@pickfire I believe that would be a significant departure from how debug_struct works, as it currently knows nothing about what fields are in the struct. As far as I can tell, what you describe would have to be supported by the derive(Debug) macro or something like that

@guswynn
Copy link
Contributor

guswynn commented Mar 24, 2021

@pickfire https://docs.rs/debug_stub_derive/0.3.0/debug_stub_derive/ may cover what you want?

JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 25, 2021
stabilize debug_non_exhaustive

tracking issue: rust-lang#67364

but it is still an open question whether the other `Debug*` struct's should have a similar method. I would guess that would best be put underneath a new feature gate, as this one seems uncontroversial enough to stabilize as is
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Mar 25, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 27, 2021
…r=jackh726

Use DebugStruct::finish_non_exhaustive() in std.

See rust-lang#67364
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 27, 2021
Improve Debug implementations of Mutex and RwLock.

This improves the Debug implementations of Mutex and RwLock.

They now show the poison flag and use debug_non_exhaustive. (See rust-lang#67364.)
@ranile
Copy link
Contributor

ranile commented Apr 25, 2021

What is preventing this from being stabilized? As far as I can tell, all the requirements are met.

@jhpratt
Copy link
Member

jhpratt commented Apr 25, 2021

@hamza1311 It's stable on nightly. It will land in Rust 1.53.

@m-ou-se
Copy link
Member

m-ou-se commented Apr 25, 2021

This was stablized a month ago: #83041

Closing this issue, since the other action items are now also completed.

@m-ou-se m-ou-se closed this as completed Apr 25, 2021
@dcormier
Copy link

I, too, would like to see this on DebugTuple, and was just surprised to find that it wasn't. I was already familiar with it on DebugStruct, so I didn't realize it was only there.

In my case, I wanted to shorten the debug representation of a tuple when any of the fields are None, and indicate that fields were left out.

@richard-uk1
Copy link
Contributor Author

If anyone wanted to have a go at writing a PR, this (implementing for DebugTuple) would be a good first issue. It can go in as unstable for experimentation, assuming that the powers that be agree on its usefulness.

@ian-h-chamberlain
Copy link
Contributor

I found this issue while wondering why there wasn't an equivalent finish_non_exhaustive for DebugTuple, but in the meantime I though I'd share a workaround I found in case people are looking for something to use on stable.

You can (ab?)use the fact that std::ops::RangeFull has a Debug impl to your advantage, and use the existing field() to display .. when you want to omit some fields:

f.debug_tuple("Foo").field(&{..}).finish()

Playground example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=d1b7f91c84969e0de54f70b90d9570bc

This may not be as nice as a first-class finish_non_exhaustive method would be, but it offers some nice flexibility and works today!

Also worth noting, perhaps, is how rust docs display private fields on tuple structs, e.g.
https://doc.rust-lang.org/std/thread/struct.JoinHandle.html

I think you could emulate this with a struct that has _ as its debug representation, but it ends up being a good chunk of boilerplate with little benefit IMO.

@richard-uk1
Copy link
Contributor Author

richard-uk1 commented Aug 31, 2022

That's a great hack! Honestly though AFAIK the only thing stopping there being an impl for this on tuples is someone writing the patch. It should be relatively straightforward even for someone who doesn't know the rustc codebase if you use this PR as a template.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-fmt Area: `std::fmt` B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests