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

Decide on stability of Display output for libstd/libcore/etc. types #72676

Open
alilleybrinker opened this issue May 27, 2020 · 8 comments
Open
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@alilleybrinker
Copy link
Contributor

Per the discussion in #62794, it's not clear whether the output of Display impls for types in the standard library should be considered stable or unstable, and if unstable, which impls might be stable.

Views expressed:

Seems worth deciding on and documenting.

@Mark-Simulacrum
Copy link
Member

I'm not sure what the best way to do this is. I don't think it is likely that we'll make progress without concrete questions -- certainly guaranteeing stability for a bunch of impls without going case-by-case seems like a bad idea.

We've also been slowly over time changing the display output with parameters given, e.g., #72407 and similar PRs which do technically break a very strict stability guarantee interpretation.

I would personally rather not explicitly guarantee anything "just because" and rather do so on a case by case basis, that is, if people ask us to. OTOH, I wouldn't expect us to change Display (or Debug, for that matter) impls with default configuration for e.g. integers at this point because that's too likely to cause breakage. But I'm not on the libs team.

@Lucretiel
Copy link
Contributor

Another related issue: #72398

@the8472
Copy link
Member

the8472 commented May 28, 2020

From the trait description:

Display is similar to Debug, but Display is for user-facing output, and so cannot be derived.

This reads like it is primarily meant for human consumption. So scriptability (which I assume is the purpose of guaranteeing stability) is secondary at best. Integers are rigid enough that it shouldn't cause too much pain to guarantee specific formatting when no additional parameters are passed and even then one could envision the idea of pretty-printing very large integers to ease readability or something like that.
Beyond that it quickly becomes less clear. Floats to string conversion is not bijective, so one could get any number of strings when printing one.
Then there's SocketAddrV6 which almost certainly is incomplete because it currently does not even print its scope ID even though it should when it's a link-local address.

At the extreme end there's Backtrace. In Java-land people occasionally try to match strings in backtraces and it results in very brittle behavior. Something like that shouldn't be encouraged.

@KamilaBorowska
Copy link
Contributor

While Display is supposed to be used for "user-facing output", the current implementations for many types implementing Display are feasibly computer-parseable, and that shouldn't change, especially because changing those will cause failures at runtime (not during compilation).

For instance, an implementation of Display for f64 shouldn't attempt to pretty-print numbers, because changing that could break code that uses .to_string() for things that are supposed to be read by computers, for instance XML files - a computer program may not understand a pretty-printed floating point number in an XML file.

However, changing Display implementations while preserving computer parsability should be fine. Changing Display for f64 from rendering 0.20000000000000001 to 0.2 is fine, because those numbers are equivalent for 64-bit floating point numbers. Bug fixes are fine, not complete changes.

@the8472
Copy link
Member

the8472 commented May 29, 2020

for instance XML files - a computer program may not understand a pretty-printed floating point number in an XML file.

Arguably you should be using an xml-serializer and xsd schemas for that purpose instead of assembling XML by juggling strings via std formatters. E.g. JSON does not support Infinity or NaN at all, while certain XSD schemas do but only with specific capitalization which Display already violates.

@KamilaBorowska
Copy link
Contributor

KamilaBorowska commented May 29, 2020

Even if you do use an XML serializer, using .to_string() sounds feasible to me (and .to_string() internally uses Display trait). In fact, serde-xml-rs uses write!(self.writer, "{}", primitive) with floating point numbers. Whether it's right or not doesn't matter, we shouldn't break currently working programs.

And it's not necessarily XML, it can be any human readable protocol really, maybe even an ad-hoc one.

@the8472
Copy link
Member

the8472 commented May 29, 2020

It probably shouldn't. Display prints inf while xsd:double requires INF. Note that XML itself does not prescribe any number formatting, so you may be in an underdefined territory where things just happen to work by chance. Cobbling things together via std formatting may be fine for prototypes and quick-and-dirty things.

But imo it's a weak argument for providing rigorous guarantees in the standard library. If you're hacking together Q&D programs then they're already expected to not be very stable / break under edge-cases. So why would it need extra guarantees?

Note that even parsing an f64 with the standard library has bugs (#31407) that probably make it unsuitable for round-tripping arbitrary data.

@camelid camelid added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Oct 20, 2020
@workingjubilee
Copy link
Member

The Display formatting of f32 and f64 should certainly not be considered for stabilization until Rust fulfills various requirements of IEEE 754 in regards to precisely that, and I think no one should be treating it as if it is perfectly stable either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. 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

8 participants