When expecting BoxFuture and using async {}, suggest Box::pin
#69082
Conversation
|
r? @davidtwco (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
estebank
force-pushed
the
boxfuture-box-pin
branch
from
d260267
to
b24d366
Compare
| | ^ | ||
| | | | ||
| | expected struct `std::boxed::Box`, found type parameter `F` | ||
| | help: store this in the heap by calling `Box::new`: `Box::new(x)` |
There was a problem hiding this comment.
This is the only part that is not ideal, and I can add some logic to avoid this but would make more complex an unrelated part of the codebase...
There was a problem hiding this comment.
Overall it seems okay for now, it seems less likely that they will start with Pin::new. And if they apply the suggestion they'll eventually be presented with a correct one.
src/libcore/marker.rs
Outdated
| @@ -727,6 +727,13 @@ unsafe impl<T: ?Sized> Freeze for &mut T {} | |||
| /// [`Pin<P>`]: ../pin/struct.Pin.html | |||
| /// [`pin module`]: ../../std/pin/index.html | |||
| #[stable(feature = "pin", since = "1.33.0")] | |||
| #[rustc_on_unimplemented( | |||
| on( | |||
| _Self = "dyn std::future::Future<Output = i32> + std::marker::Send", | |||
There was a problem hiding this comment.
I don't think you specifically want i32 here?
There was a problem hiding this comment.
Oh damn, you're right, sigh.
There was a problem hiding this comment.
I had to modify rustc_on_unimplemented for this to work, but won't be able to test it until those changes get picked up by beta.
There was a problem hiding this comment.
For clarification, the note still appears, it just isn't being tested for regressions. I will reenable those tests after the next release is cut.
There was a problem hiding this comment.
That's fine with me, but I don't really understand why we have to wait for beta.
There was a problem hiding this comment.
Not sure I understand either, but I remember having the same issue sometime back when extending rustc_on_unimplemented.
| | ^ | ||
| | | | ||
| | expected struct `std::boxed::Box`, found type parameter `F` | ||
| | help: store this in the heap by calling `Box::new`: `Box::new(x)` |
There was a problem hiding this comment.
Overall it seems okay for now, it seems less likely that they will start with Pin::new. And if they apply the suggestion they'll eventually be presented with a correct one.
estebank
force-pushed
the
boxfuture-box-pin
branch
from
b24d366
to
b8eb81c
Compare
|
r? @tmandry |
This comment has been minimized.
This comment has been minimized.
estebank
force-pushed
the
boxfuture-box-pin
branch
from
b8eb81c
to
248f5a4
Compare
|
@bors r+ |
|
📌 Commit 248f5a4 has been approved by |
bors
added
S-waiting-on-bors
When expecting `BoxFuture` and using `async {}`, suggest `Box::pin`
Fix rust-lang#68197, cc rust-lang#69083.
Rollup of 9 pull requests Successful merges: - #67642 (Relax bounds on HashMap/HashSet) - #68848 (Hasten macro parsing) - #69008 (Properly use parent generics for opaque types) - #69048 (Suggestion when encountering assoc types from hrtb) - #69049 (Optimize image sizes) - #69050 (Micro-optimize the heck out of LEB128 reading and writing.) - #69068 (Make the SGX arg cleanup implementation a NOP) - #69082 (When expecting `BoxFuture` and using `async {}`, suggest `Box::pin`) - #69104 (bootstrap: Configure cmake when building sanitizer runtimes) Failed merges: r? @ghost
Fix #68197, cc #69083.