Uh oh!
There was an error while loading. Please reload this page.
Cleanup formatting code - #67795
Conversation
rust-highfive
commented
Jan 2, 2020
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
commented
Jan 2, 2020
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Mark-Simulacrum
commented
Jan 2, 2020
Test failure here looks expected -- it looks like |
Mark-Simulacrum
commented
Jan 19, 2020
dtolnay
left a comment
There was a problem hiding this comment.
The code change looks good to me. Thanks! This is great.
Regarding the test failure -- the expected failure that is no longer failing is:
rust/src/test/ui/async-await/async-fn-nonsend.rs
Lines 39 to 45 in 6250d56
I don't think your assessment that fmt::Formatter is now Send and maybe Sync is accurate, since it still contains a &mut dyn Write which is neither; https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=e02471e6cddb6a31241f2089e714a8da. Could you dig a bit deeper to find out what change in this PR is affecting the test? I would prefer not to have an autotrait impl appear unintentionally that we might have trouble preserving down the line.
If the same error from the test case just got bumped to a later "phase" of errors from rustc for some reason, then that's fine and we can move forward.
Mark-Simulacrum
commented
Jan 20, 2020
I will try to bisect or something like that (at least run the test locally and play around with it). I agree with your assessment that Send/Sync should not have been affected due to the continued presence of |
The formatting infrastructure stopped emitting these a while back, and in removing them we can simplify related code.
These are only called from one place and don't generally support being called from other places; furthermore, they're the only formatter functions that look at the `args` field (which a future commit will remove).
These are no longer used by Formatter methods.
fmt::Formatter is still not Send/Sync, but the UI test emitted two errors, for the dyn Write and the Void inside Formatter. As of this PR, the Void is now gone, but the dyn Write remains.
8de4b81 to
a804a45CompareMark-Simulacrum
commented
Jan 20, 2020
Pushed up one more commit (as well as rebasing, though there were no conflicts) which fixes the test in question. Turns out there was just a "duplicate" error (both for Void and the dyn Write), with the first of those being gone. This is ready for review again (I'm not sure if you meant to say r=me in your comment). r? @dtolnay |
dtolnay
commented
Jan 20, 2020
Nice, I'm glad it turned out to be something simple. @Mark-Simulacrum it could be worth adding @bors r+ |
bors
commented
Jan 20, 2020
📌 Commit a804a45 has been approved by |
Mark-Simulacrum
commented
Jan 20, 2020
(I wonder if it makes sense to change the async/await error messages to not "look into" external types by default -- I think there are cases where you'd want to, but generally speaking it seems like " |
…lnay Cleanup formatting code This removes a few leftover positional enum variants that were no longer used. All details that are changed are unstable (and `#[doc(hidden)]`), so this should not impact downstream code.
Rollup of 8 pull requests Successful merges: - #67734 (Remove appendix from Apache license) - #67795 (Cleanup formatting code) - #68290 (Fix some tests failing in `--pass check` mode) - #68297 ( Filter and test predicates using `normalize_and_test_predicates` for const-prop) - #68302 (Fix #[track_caller] and function pointers) - #68339 (Add `riscv64gc-unknown-linux-gnu` into target list in build-manifest) - #68381 (Added minor clarification to specification of GlobalAlloc::realloc.) - #68397 (rustdoc: Correct order of `async` and `unsafe` in `async unsafe fn`s) Failed merges: r? @ghost
This removes a few leftover positional enum variants that were no longer used.
All details that are changed are unstable (and
#[doc(hidden)]), so this shouldnot impact downstream code.