core: split into fmt::Show and fmt::String #20481
Conversation
|
r=me |
| @@ -581,7 +581,7 @@ impl<T: Eq> Eq for Arc<T> {} | |||
|
|
|||
| impl<T: fmt::Show> fmt::Show for Arc<T> { | |||
| fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | |||
| (**self).fmt(f) | |||
| write!(f, "Arc {{ {:?} }}", (**self)) | |||
There was a problem hiding this comment.
This may want to be consistent with Box below and use Arc({:?}) instead (or change the Box one below
There was a problem hiding this comment.
They're different since Box is a tuple struct, and Arc and Rc aren't. I can make then equivalent, though.
There was a problem hiding this comment.
Yeah let's keep them consistent for now.
|
Awesome work, thanks @seanmonstar! Just a few nits but otherwise this looks great to me. |
|
Can you also elaborate the commit message in accordance with our breaking changes policy? |
seanmonstar
force-pushed
the
fmt-show-string
branch
from
d1ec1aa
to
fbf7e73
Compare
|
Fixed up most nits, lacking the commit message elaboration. Also, I'm seeing a couple of ICEs in run-pass tests, namely in |
|
Needs another rebase, and I think that error means that an associated type was left out of a trait implementation that should have otherwise been included. |
seanmonstar
force-pushed
the
fmt-show-string
branch
2 times, most recently
from
b78b19e
to
58febcc
Compare
|
@alexcrichton rebased just now to master, and fixed the commit message. I'm currently running make check, but my machine is slow :( |
fmt::Show is for debugging, and can and should be implemented for
all public types. This trait is used with `{:?}` syntax. There still
exists #[derive(Show)].
fmt::String is for types that faithfully be represented as a String.
Because of this, there is no way to derive fmt::String, all
implementations must be purposeful. It is used by the default format
syntax, `{}`.
This will break most instances of `{}`, since that now requires the type
to impl fmt::String. In most cases, replacing `{}` with `{:?}` is the
correct fix. Types that were being printed specifically for users should
receive a fmt::String implementation to fix this.
Part of rust-lang#20013
[breaking-change]
seanmonstar
force-pushed
the
fmt-show-string
branch
from
58febcc
to
44440e5
Compare
Conflicts: src/compiletest/runtest.rs src/libcore/fmt/mod.rs src/libfmt_macros/lib.rs src/libregex/parse.rs src/librustc/middle/cfg/construct.rs src/librustc/middle/dataflow.rs src/librustc/middle/infer/higher_ranked/mod.rs src/librustc/middle/ty.rs src/librustc_back/archive.rs src/librustc_borrowck/borrowck/fragments.rs src/librustc_borrowck/borrowck/gather_loans/mod.rs src/librustc_resolve/lib.rs src/librustc_trans/back/link.rs src/librustc_trans/save/mod.rs src/librustc_trans/trans/base.rs src/librustc_trans/trans/callee.rs src/librustc_trans/trans/common.rs src/librustc_trans/trans/consts.rs src/librustc_trans/trans/controlflow.rs src/librustc_trans/trans/debuginfo.rs src/librustc_trans/trans/expr.rs src/librustc_trans/trans/monomorphize.rs src/librustc_typeck/astconv.rs src/librustc_typeck/check/method/mod.rs src/librustc_typeck/check/mod.rs src/librustc_typeck/check/regionck.rs src/librustc_typeck/collect.rs src/libsyntax/ext/format.rs src/libsyntax/ext/source_util.rs src/libsyntax/ext/tt/transcribe.rs src/libsyntax/parse/mod.rs src/libsyntax/parse/token.rs src/test/run-pass/issue-8898.rs
r? @alexcrichton
Each time I rebase, it breaks again, as there are new commits that use
{}for debugging. So keeping this up-to-date has been tricky. I'm not certain that the current commit passesmake check, but I figured I'd put it up for review while I let the build run.Part of #20013