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.