Skip to content

Debug impl of Any could be improved #46261

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

Closed
mzabaluev opened this issue Nov 25, 2017 · 10 comments
Closed

Debug impl of Any could be improved #46261

mzabaluev opened this issue Nov 25, 2017 · 10 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@mzabaluev
Copy link
Contributor

The Debug impl of Any is currently very simple, it always returns "Any":

use std::any::Any;

/// I don't care if this downcasts to a string,
/// it's all "Any" to me.
fn print_any(something: &Any) {
    println!("{:?}", something);
}

fn main() {
    print_any(&String::from("Hello"));
}

This shows up in panic messages when e.g. unwrap() is called on a thread::Result of a panicked thread. The implementation could do what the default panic hook already does and try to downcast the reference to &'static str and String (and maybe also Cow<'static, str>, though that might be very unusual) and, if successful, include the contents into the output or simply delegate to the string's Debug.

@bluss bluss added C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 25, 2017
@bluss
Copy link
Member

bluss commented Nov 25, 2017

panic! and assert! produce (usually) &str and String, so those would be good to cover.

@mzabaluev
Copy link
Contributor Author

mzabaluev commented Nov 25, 2017

I could try to create a pull request. What format does the audience prefer? Delegate to the strings' Debug, or should Any get a mention in the output? Box delegates, and I think Any should, too.

@mzabaluev
Copy link
Contributor Author

One problem is that String is an allocating container defined in liballoc, so it can't be known to the impls in libcore without some low-level hacking. A Debug implementation that only downcasts to static strings would leave out any dynamically formatted panic messages and would look more inconsistent than it does now.

mzabaluev added a commit to mzabaluev/rust that referenced this issue Nov 25, 2017
It would be nice if Any would write contents of any standard strings
it contained in the implementation of Debug. Unfortunately, it
cannot be done without hacking dynamic type resolution, since many
strings that are typically dumped as Any exist as owned String values
and String is up the foodchain from libcore where Any is defined.

See rust-lang#46261 for some discussion.
@mzabaluev
Copy link
Contributor Author

mzabaluev commented Nov 25, 2017

So perhaps, the String downcasting protocol could be defined in libcore to be filled in by the implementation in alloc or stubs in no-alloc linkages, similar to how it's done with panicking. Basically, it's one callback for checking against a type id, and another for obtaining the string slice from an Any pointer.

@abonander
Copy link
Contributor

More discussion here: rust-lang/rfcs#1389

@mzabaluev
Copy link
Contributor Author

@abonander Thank you, I failed to search in the RFC issues.
The powers that be can close this Rust issue now, or maybe keep it around as the tracking issue for an eventual RFC.

@abonander
Copy link
Contributor

If we're just talking about changing the internals of an impl, I don't think it needs an RFC. I think most people would be happy with it printing the value if the internal type is String or &str.

@mzabaluev
Copy link
Contributor Author

mzabaluev commented Nov 26, 2017

The RFC-qualifying issue is that, as libcore does not know anything about owned types, it would need a special-case hook for what liballoc, or conceivably another library in the program's runtime stack, would supply as the typecheck and as_str interface for String. The Debug impl that only prints static panic values would be a glass half empty.

@abonander
Copy link
Contributor

abonander commented Nov 26, 2017

I can't think of a way that wouldn't be incredibly hacky or violate coherence. I think a specialized impl Debug for Box<Any> which can downcast to String would be a good place to start, since both types are in alloc. This would lift the existing code out of the panic framework that's already doing the same.

@Mark-Simulacrum
Copy link
Member

I'm going to go ahead and close this as it seems like it's quite hard to do in practice (due to not having the relevant types around). A PR for this would be the best way to land changes here, and discussion can happen on internals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. 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

4 participants