Improve the documentation of Display and FromStr, and their interactions

[joshtriplett]

In particular:

• Display is not necessarily lossless
• The output of Display might not be parseable by FromStr, and might
  not produce the same value if it is.
• Calling .parse() on the output of Display is usually a mistake
  unless a type's documented output and input formats match.
• The input formats accepted by FromStr depend on the type.

This documentation adds no API surface area and makes no guarantees about stability. To the best of my knowledge, everything it says is already established to be true. As such, I don't think it needs an FCP.

[rustbot]

r? @scottmcm

rustbot has assigned @scottmcm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[BurntSushi]

I think I generally agree here, but I do think making Display and FromStr match is good practice where possible. Maybe we can tweak the language here to accommodate that?

[joshtriplett]

@BurntSushi wrote:

I think I generally agree here, but I do think making Display and FromStr match is good practice where possible. Maybe we can tweak the language here to accommodate that?

I'm not sure if even a description of "good practice" matches widespread community practice. "Where possible" excludes a large number of things, including things like Path/PathBuf, OsStr/OsString, BStr/BString, and many types whose Display impls just aren't meant for round-tripping at all; the users of many types should typically be using different mechanisms to output and input those types losslessly. In particular, I don't think we should imply that people "should" make a Display impl that's meant for round-tripping rather than for human consumption. It really depends on the type, which is what the documentation here says.

In addition to that, a Display implementation also doesn't make any guarantees about valid delimiters; even if FromStr works on the exact output of Display, it also requires type-specific knowledge to store and extract that value as part of some larger data using any method other than a length or a universal escaping mechanism. Again, that's going to be up to the type and its documentation.

That said, I can attempt to tweak the language here, without making any new guarantees.

[BurntSushi]

I think there might be a misunderstanding? I'm saying that if your type provides both Display and FromStr impls, then it's good practice to make them compatible with one another. If your type lacks one or both of the impls, then I agree, the advice doesn't apply. (As is the case for Path, OsStr, ByteStr and so on.)

I agree about the delimiters and what not. But I think that's a different can of worms. :-)

[ChrisDenton]

If your type lacks one or both of the impls

Aside: Lacking only Display is strongly discouraged by the docs: https://doc.rust-lang.org/std/string/trait.ToString.html

[ToString] is automatically implemented for any type which implements the Display trait. As such, ToString shouldn’t be implemented directly: Display should be implemented instead, and you get the ToString implementation for free.

Personally I don't love the tight coupling between Display and ToString but there's no use complaining about it now and I'll admit it can be convenient.

[joshtriplett]

I added a note that "Having both Display and FromStr implementations where the result of Display cannot be parsed with FromStr may surprise users.", along with the qualifier that that doesn't mean the result of parsing will have exactly the same value as the input.

[BurntSushi]

Thanks! I'm happy with that.

[joshtriplett]

Thanks! I'm happy with that.

(Is that an r= or would you prefer someone else to review? :) )

[BurntSushi]

I did a bit more of a careful review. I think the phrasing here is still a bit too pessimistic for my liking.

[joshtriplett]

I did a bit more of a careful review. I think the phrasing here is still a bit too pessimistic for my liking.

Apparently my attempt to push the change I mentioned in #136687 (comment) failed. Hopefully that helps.

[joshtriplett]

@BurntSushi I think I've addressed your concern here; please let me know if the current version doesn't address your concern.

[scottmcm]

This feels weaker than I'd expect, TBH. I'd have expected to see a "if you implement both, then you should make the display parsable, ideally to an equivalent (but perhaps not identical) value".

Not a "must", but more than a "if you feel like it I guess you could".

Maybe it'd be worth saying something more directly that this isn't a serialization API, and that people wanting that should check crates.io?

TBH, the amount of back-and-forth makes me think the text here might as well go through a libs-api FCP, to give it more weight. Otherwise it has to be really vague and not really say anything.

[the8472]

One example in the standard library where a type implements both Display and FromStr but doesn't roundtrip is SocketAddrV6. The flowinfo and possibly the scopeinfo aren't preserved.

[Amanieu]

Reading through this thread, it seems that the discussion fundamentally boils down to 2 questions:

• On a type that implements both Display and FromStr, can a user to assume that the FromStr implementation can parse the output of the Display implementation?

• If yes then is the Display implementation guaranteed to be complete? This implies that displaying and parsing is able to fully preserve the value.

Obviously a type may "override" these guarantees with documentation. We only need to specify the default expectations in the absence of specific documentation to the contrary.

---

I believe that the answer to the first question is yes: the default expectation should be that FromStr is able to parse the format that is output by a Display impl. However the second question is trickier and I think it would be reasonable to warn users against expecting round-tripping unless specifically promised by type documentation.

[ijackson]

Having Display just display things seems justifiable. But ToString::to_string being lossy is much more surprising. Indeed it's arguably a violation of the albeit unofficial Rust API Guidelines.

Maybe in the 2027 edition we can have edition-specific traits (I'm looking at you, std::error::Error) somehow and then we can have a version of .to_string() that only works if the type roundtrips through String.

Or we could rename ToString::to_string to ToDisplayString::to_display_string and have a new ToFaithfulString::to_string implemented for only types that opt in somehow.

Both of these would mean a semantic difference between format("{x}") and x.to_string_sensibly() but that seems fair to me.

[ijackson]

Having Display just display things seems justifiable. But ToString::to_string being lossy is much more surprising. Indeed it's arguably a violation of the albeit unofficial Rust API Guidelines.

So err maybe we could add some docs to ToString in the meantime, where we apologise for this beartrap?

[jdahlstrom]

But ToString::to_string being lossy is much more surprising. Indeed it's arguably a violation of the albeit unofficial Rust API Guidelines.

I don't see how. The guidelines don't say or recommend that to_ conversions should be injective.

---

In a type that implements both Display and FromStr, can a user to assume that the FromStr implementation can parse the output of the Display implementation?

My 2c is that from_str(&to_string(x)) == Ok(y) should be guaranteed for at least all primitive types, and additionally that x == y wherever possible. Currently FromStr for f32 documents the grammar that it accepts (including ±Inf and NaN), but Display for f32 doesn't specify what it outputs, although in practice I'd expect that all f32 bit patterns at least fulfill the first condition. Integer types don't specify anything about how they format/parse strings, but I presume that they "obviously" round-trip.