Skip to content

Improve the documentation of Display and FromStr, and their interactions #136687

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions library/core/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,15 @@ pub use macros::Debug;
/// [tostring]: ../../std/string/trait.ToString.html
/// [tostring_function]: ../../std/string/trait.ToString.html#tymethod.to_string
///
/// # Completeness and parseability
///
/// `Display` for a type might not necessarily be a lossless or complete representation of the type.
/// It may omit internal state, precision, or other information the type does not consider important
/// for user-facing output, as determined by the type. As such, the output of `Display` might not be
/// possible to parse, and even if it is, the result of parsing might not exactly match the original
/// value. Calling `.parse()` on the output from `Display` is usually a mistake, unless the type has
/// provided and documented additional guarantees about its `Display` and `FromStr` implementations.
///
/// # Internationalization
///
/// Because a type can only have one `Display` implementation, it is often preferable
Expand Down
15 changes: 15 additions & 0 deletions library/core/src/str/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,21 @@ unsafe impl SliceIndex<str> for ops::RangeToInclusive<usize> {
/// parse an `i32` with `FromStr`, but not a `&i32`. You can parse a struct that
/// contains an `i32`, but not one that contains an `&i32`.
///
/// # Input format
///
/// The input format expected by a type's `FromStr` implementation depends on the type. Check the
/// type's documentation for the input formats it knows how to parse. Note that the input format of
/// a type's `FromStr` implementation might not necessarily accept the output format of its
/// `Display` implementation; thus, calling `.parse()` on the output from `Display` is usually a
/// mistake, unless the type has provided and documented additional guarantees about its `Display`
/// and `FromStr` implementations.
///
/// If a type happens to have a lossless `Display` implementation whose output is meant to be
/// conveniently machine-parseable and not just meant for human consumption, then the type may wish
/// to accept the same format in `FromStr`, and document that usage. Having both `Display` and
/// `FromStr` implementations where the result of `Display` cannot be parsed with `FromStr` may
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

/// surprise users. (However, the result of such parsing may not have the same value as the input.)
///
/// # Examples
///
/// Basic implementation of `FromStr` on an example `Point` type:
Expand Down
Loading