Skip to content

Document proper usage of fmt::Error and fmt()'s Result. #124954

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
May 11, 2024

Conversation

kpreid
Copy link
Contributor

@kpreid kpreid commented May 10, 2024

I've seen several newcomers wonder why fmt::Error doesn't have any error detail information, or propose to return it in response to an error condition found inside a impl fmt::Display for MyType.

That is incorrect, per a lone paragraph of the fmt module's documentation. However, users looking to implement a formatting trait won't necessarily look there. Therefore, let's add the critical information (that formatting per se is infallible) to all the involved items: every fmt() method, and fmt::Error.

This PR is not intended to make any novel claims about fmt; only to repeat an existing one in places where it will be more visible.

Documentation of these properties previously existed in a lone paragraph
in the `fmt` module's documentation:
<https://doc.rust-lang.org/1.78.0/std/fmt/index.html#formatting-traits>
However, users looking to implement a formatting trait won't necessarily
look there. Therefore, let's add the critical information (that
formatting per se is infallible) to all the involved items.
@rustbot
Copy link
Collaborator

rustbot commented May 10, 2024

r? @Nilstrieb

rustbot has assigned @Nilstrieb.
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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 10, 2024
@Noratrieb
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented May 11, 2024

📌 Commit c21c5ba has been approved by Nilstrieb

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2024
@RalfJung
Copy link
Member

RalfJung commented May 11, 2024

I wonder if this should be made into a panic then, since it can't be reached unless there's a bug in the formatter?

@Noratrieb
Copy link
Member

that should indeed be unreachable unless someone has a fallible formatting impl. not sure whether it's worth panicking in that case though, maybe.

@RalfJung
Copy link
Member

We do panic in the equivalent case in format!.

bors added a commit to rust-lang-ci/rust that referenced this pull request May 11, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#124928 (Stabilize `byte_slice_trim_ascii` for `&[u8]`/`&str`)
 - rust-lang#124954 (Document proper usage of `fmt::Error` and `fmt()`'s `Result`.)
 - rust-lang#124969 (check if `x test tests` missing any test directory)
 - rust-lang#124978 (Handle Deref expressions in invalid_reference_casting)
 - rust-lang#125005 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6c3fce9 into rust-lang:master May 11, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 11, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 11, 2024
Rollup merge of rust-lang#124954 - kpreid:fmterr, r=Nilstrieb

Document proper usage of `fmt::Error` and `fmt()`'s `Result`.

I've seen several newcomers wonder why `fmt::Error` doesn't have any error detail information, or propose to return it in response to an error condition found inside a `impl fmt::Display for MyType`.

That is incorrect, per [a lone paragraph of the `fmt` module's documentation](https://doc.rust-lang.org/1.78.0/std/fmt/index.html#formatting-traits). However, users looking to implement a formatting trait won't necessarily look there. Therefore, let's add the critical information (that formatting per se is infallible) to all the involved items: every `fmt()` method, and `fmt::Error`.

This PR is not intended to make any novel claims about `fmt`; only to repeat an existing one in places where it will be more visible.
@kpreid

This comment was marked as outdated.

@kpreid kpreid deleted the fmterr branch May 11, 2024 17:22
bors added a commit to rust-lang-ci/rust that referenced this pull request May 12, 2024
…crum,workingjubilee

io::Write::write_fmt: panic if the formatter fails when the stream does not fail

Follow-up to rust-lang#124954
RalfJung pushed a commit to RalfJung/miri that referenced this pull request May 12, 2024
…ingjubilee

io::Write::write_fmt: panic if the formatter fails when the stream does not fail

Follow-up to rust-lang/rust#124954
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants