#74: Debug impl forwards through Debug, not Display#75
Merged
Conversation
Owner
|
@guaardvark don't commit |
guaardvark
force-pushed
the
fix/74-debug-forwards-debug
branch
from
00e3f49 to
17dca3c
Compare
Contributor
Author
|
Good catch, sorry about that — removed the |
guaardvark
force-pushed
the
fix/74-debug-forwards-debug
branch
2 times, most recently
from
56ffb91 to
232405c
Compare
The `Debug` impl for `Stack<V, N>` was bounded on `V: Display + Copy` and
formatted each element with `format!("{v}")`, i.e. via `Display::fmt`. As a
result a `Stack<T, N>` whose element only implements `Debug` (any plain
`#[derive(Debug)]` type, `Option<_>`, etc.) could not be printed with `{:?}`
at all, and when `T` implemented both traits the `{:?}` output silently
showed the `Display` form instead of the `Debug` form.
This contradicts every std container, where `Debug` on the container
forwards through `Debug` on the element. Bound the `Debug` impl on
`V: Debug + Copy` and format with `format!("{v:?}")`.
The `Display` impl previously delegated to `Debug`; since the two impls now
have different bounds and different output, `Display` gets its own identical
loop over `Display::fmt`, keeping its behaviour unchanged.
Tests: `debugs_stack` now asserts the quoted `Debug` form of `&str`; a new
`debugs_stack_of_debug_only_type` uses `Stack<Option<i32>, N>` (Debug but
not Display) which would not have compiled under the old bound.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #74.
Problem
The
Debugimpl forStack<V, N>(src/debug.rs) was bounded onV: Display + Copyand formatted each element withformat!("{v}")— that callsDisplay::fmt, notDebug::fmt. Two consequences:Stack<T, N>whose element implementsDebugbut notDisplay(any plain#[derive(Debug)]struct,Option<_>, etc.) could not be printed with{:?}at all, even thoughT: Debugholds.Timplemented both traits,{:?}silently produced theDisplayrepresentation instead of theDebugone.This contradicts the convention every
stdcontainer follows, whereDebugon the container forwards throughDebugon the element.Fix
Bound the
Debugimpl onV: Debug + Copyand format withformat!("{v:?}").The
Displayimpl previously delegated toDebug(<&Self as Debug>::fmt). Since the two impls now carry different bounds (DisplayvsDebug) and produce different output, that delegation no longer type-checks, soDisplaygets its own identical loop overDisplay::fmt— its behaviour is unchanged.Tests
debugs_stacknow asserts the quotedDebugform of&str:[\"one\", \"two\"].debugs_stack_of_debug_only_typebuilds aStack<Option<i32>, N>—Option<i32>isDebug+Copybut notDisplay, so this test would not even have compiled under the old bound.Verified locally:
cargo test(debug + release),cargo fmt --check, andcargo clippy -- --no-depsall clean, with no new warnings indebug.rs.