Clarify Display for error should not include source #146679
Conversation
rustbot
added
S-waiting-on-review
|
|
|
r? libs-api |
rustbot
added
the
T-libs-api
library/core/src/error.rs
Outdated
| /// `Display` for an error should not typically include information from `Error::source()`: | ||
| /// error stack can be printed by external utilities. |
There was a problem hiding this comment.
Standard library documentation should articulate how the standard library is intended to be used. To me "error stack can be printed by external utilities" reads like "authors of external utilities will intuit the intended way to work with this API" which is less than clear if the reader is the person wanting to write external utilities, and less than correct in implying that the use of an external utility is necessary for anyone to whom the correct way is not intuitive.
Maybe something along these lines: "In error types that wrap an underlying error, the underlying error should either be returned by the outer error's Error::source(), or rendered by the outer error's Display, but not both. Doing both will cause duplication of that error message by code that traverse the chain of causes of an error to print each one, which is typical in error reporting utilities."
rustbot
added
S-waiting-on-author
stepancheg
force-pushed
the
error-display-source
branch
from
8a12ccf to
a9554b4
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
rustbot
added
S-waiting-on-review
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
Rollup of 7 pull requests Successful merges: - #146556 (Fix duration_since panic on unix when std is built with integer overflow checks) - #146679 (Clarify Display for error should not include source) - #146753 (Improve the pretty print of UnstableFeature clause) - #146894 (Improve derive suggestion of const param) - #146950 (core: simplify `CStr::default()`) - #146958 (Fix infinite recursion in Path::eq with String) - #146971 (fix ICE in writeback due to bound regions) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 7 pull requests Successful merges: - #146556 (Fix duration_since panic on unix when std is built with integer overflow checks) - #146679 (Clarify Display for error should not include source) - #146753 (Improve the pretty print of UnstableFeature clause) - #146894 (Improve derive suggestion of const param) - #146950 (core: simplify `CStr::default()`) - #146958 (Fix infinite recursion in Path::eq with String) - #146971 (fix ICE in writeback due to bound regions) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 7 pull requests Successful merges: - rust-lang/rust#146556 (Fix duration_since panic on unix when std is built with integer overflow checks) - rust-lang/rust#146679 (Clarify Display for error should not include source) - rust-lang/rust#146753 (Improve the pretty print of UnstableFeature clause) - rust-lang/rust#146894 (Improve derive suggestion of const param) - rust-lang/rust#146950 (core: simplify `CStr::default()`) - rust-lang/rust#146958 (Fix infinite recursion in Path::eq with String) - rust-lang/rust#146971 (fix ICE in writeback due to bound regions) r? `@ghost` `@rustbot` modify labels: rollup
Fixes #145561.
r? @Noratrieb