Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggest Box::pin when Pin::new is used instead #89870

Merged
merged 6 commits into from
Oct 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions compiler/rustc_typeck/src/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::{Coercion, InferOk, InferResult};
use rustc_infer::traits::Obligation;
use rustc_infer::traits::{Obligation, TraitEngine, TraitEngineExt};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::adjustment::{
Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCast,
Expand Down Expand Up @@ -146,6 +146,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
.and_then(|InferOk { value: ty, obligations }| success(f(ty), ty, obligations))
}

#[instrument(skip(self))]
fn coerce(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> CoerceResult<'tcx> {
// First, remove any resolved type variables (at the top level, at least):
let a = self.shallow_resolve(a);
Expand Down Expand Up @@ -933,14 +934,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

/// Same as `try_coerce()`, but without side-effects.
///
/// Returns false if the coercion creates any obligations that result in
/// errors.
pub fn can_coerce(&self, expr_ty: Ty<'tcx>, target: Ty<'tcx>) -> bool {
let source = self.resolve_vars_with_obligations(expr_ty);
debug!("coercion::can({:?} -> {:?})", source, target);
debug!("coercion::can_with_predicates({:?} -> {:?})", source, target);

let cause = self.cause(rustc_span::DUMMY_SP, ObligationCauseCode::ExprAssignable);
// We don't ever need two-phase here since we throw out the result of the coercion
let coerce = Coerce::new(self, cause, AllowTwoPhase::No);
self.probe(|_| coerce.coerce(source, target)).is_ok()
self.probe(|_| {
let ok = match coerce.coerce(source, target) {
Ok(ok) => ok,
_ => return false,
};
let mut fcx = traits::FulfillmentContext::new_in_snapshot();
fcx.register_predicate_obligations(self, ok.obligations);
fcx.select_where_possible(&self).is_ok()
})
}

/// Given a type and a target type, this function will calculate and return
Expand Down
97 changes: 71 additions & 26 deletions compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def::{CtorOf, DefKind};
use rustc_hir::lang_items::LangItem;
use rustc_hir::{Expr, ExprKind, ItemKind, Node, Stmt, StmtKind};
use rustc_hir::{Expr, ExprKind, ItemKind, Node, Path, QPath, Stmt, StmtKind, TyKind};
use rustc_infer::infer;
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, Binder, Ty};
use rustc_span::symbol::kw;
use rustc_span::symbol::{kw, sym};

use std::iter;

Expand Down Expand Up @@ -341,12 +341,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
for (sp, label) in spans_and_labels {
multi_span.push_span_label(sp, label);
}
err.span_note(multi_span, "closures can only be coerced to `fn` types if they do not capture any variables");
err.span_note(
multi_span,
"closures can only be coerced to `fn` types if they do not capture any variables"
);
}
}
}

/// When encountering an `impl Future` where `BoxFuture` is expected, suggest `Box::pin`.
#[instrument(skip(self, err))]
pub(in super::super) fn suggest_calling_boxed_future_when_appropriate(
&self,
err: &mut DiagnosticBuilder<'_>,
Expand All @@ -361,33 +365,74 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return false;
}
let pin_did = self.tcx.lang_items().pin_type();
match expected.kind() {
ty::Adt(def, _) if Some(def.did) != pin_did => return false,
// This guards the `unwrap` and `mk_box` below.
_ if pin_did.is_none() || self.tcx.lang_items().owned_box().is_none() => return false,
_ => {}
// This guards the `unwrap` and `mk_box` below.
if pin_did.is_none() || self.tcx.lang_items().owned_box().is_none() {
return false;
}
let boxed_found = self.tcx.mk_box(found);
let new_found = self.tcx.mk_lang_item(boxed_found, LangItem::Pin).unwrap();
if self.can_coerce(new_found, expected) {
match found.kind() {
ty::Adt(def, _) if def.is_box() => {
err.help("use `Box::pin`");
let box_found = self.tcx.mk_box(found);
let pin_box_found = self.tcx.mk_lang_item(box_found, LangItem::Pin).unwrap();
let pin_found = self.tcx.mk_lang_item(found, LangItem::Pin).unwrap();
match expected.kind() {
ty::Adt(def, _) if Some(def.did) == pin_did => {
if self.can_coerce(pin_box_found, expected) {
debug!("can coerce {:?} to {:?}, suggesting Box::pin", pin_box_found, expected);
match found.kind() {
ty::Adt(def, _) if def.is_box() => {
err.help("use `Box::pin`");
}
_ => {
err.multipart_suggestion(
"you need to pin and box this expression",
vec![
(expr.span.shrink_to_lo(), "Box::pin(".to_string()),
(expr.span.shrink_to_hi(), ")".to_string()),
],
Applicability::MaybeIncorrect,
);
}
}
true
} else if self.can_coerce(pin_found, expected) {
match found.kind() {
ty::Adt(def, _) if def.is_box() => {
err.help("use `Box::pin`");
true
}
_ => false,
}
} else {
false
}
_ => {
err.multipart_suggestion(
"you need to pin and box this expression",
vec![
(expr.span.shrink_to_lo(), "Box::pin(".to_string()),
(expr.span.shrink_to_hi(), ")".to_string()),
],
Applicability::MachineApplicable,
);
}
ty::Adt(def, _) if def.is_box() && self.can_coerce(box_found, expected) => {
// Check if the parent expression is a call to Pin::new. If it
// is and we were expecting a Box, ergo Pin<Box<expected>>, we
// can suggest Box::pin.
let parent = self.tcx.hir().get_parent_node(expr.hir_id);
let fn_name = match self.tcx.hir().find(parent) {
Some(Node::Expr(Expr { kind: ExprKind::Call(fn_name, _), .. })) => fn_name,
_ => return false,
};
match fn_name.kind {
ExprKind::Path(QPath::TypeRelative(
hir::Ty {
kind: TyKind::Path(QPath::Resolved(_, Path { res: recv_ty, .. })),
..
},
method,
)) if Some(recv_ty.def_id()) == pin_did && method.ident.name == sym::new => {
err.span_suggestion(
fn_name.span,
"use `Box::pin` to pin and box this expression",
"Box::pin".to_string(),
Applicability::MachineApplicable,
);
true
}
_ => false,
}
}
true
} else {
false
_ => false,
}
}

Expand Down
6 changes: 2 additions & 4 deletions src/test/ui/cross/cross-borrow-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ error[E0308]: mismatched types
--> $DIR/cross-borrow-trait.rs:10:26
|
LL | let _y: &dyn Trait = x;
| ---------- ^
| | |
| | expected `&dyn Trait`, found struct `Box`
| | help: consider borrowing here: `&x`
| ---------- ^ expected `&dyn Trait`, found struct `Box`
| |
| expected due to this
|
= note: expected reference `&dyn Trait`
Expand Down
12 changes: 4 additions & 8 deletions src/test/ui/dst/dst-bad-coercions.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ error[E0308]: mismatched types
--> $DIR/dst-bad-coercions.rs:15:21
|
LL | let y: &dyn T = x;
| ------ ^
| | |
| | expected `&dyn T`, found *-ptr
| | help: consider borrowing here: `&x`
| ------ ^ expected `&dyn T`, found *-ptr
| |
| expected due to this
|
= note: expected reference `&dyn T`
Expand All @@ -37,10 +35,8 @@ error[E0308]: mismatched types
--> $DIR/dst-bad-coercions.rs:20:21
|
LL | let y: &dyn T = x;
| ------ ^
| | |
| | expected `&dyn T`, found *-ptr
| | help: consider borrowing here: `&x`
| ------ ^ expected `&dyn T`, found *-ptr
| |
| expected due to this
|
= note: expected reference `&dyn T`
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/suggestions/box-future-wrong-output.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Issue #72117
// edition:2018

use core::future::Future;
use core::pin::Pin;

pub type BoxFuture<'a, T> = Pin<Box<dyn Future<Output = T> + Send + 'a>>;

impl<T: ?Sized> FutureExt for T where T: Future {}
trait FutureExt: Future {
fn boxed<'a>(self) -> BoxFuture<'a, Self::Output>
where
Self: Sized + Send + 'a,
{
Box::pin(self)
}
}

fn main() {
let _: BoxFuture<'static, bool> = async {}.boxed();
//~^ ERROR: mismatched types
}
14 changes: 14 additions & 0 deletions src/test/ui/suggestions/box-future-wrong-output.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0308]: mismatched types
--> $DIR/box-future-wrong-output.rs:20:39
|
LL | let _: BoxFuture<'static, bool> = async {}.boxed();
| ------------------------ ^^^^^^^^^^^^^^^^ expected `bool`, found `()`
| |
| expected due to this
|
= note: expected struct `Pin<Box<(dyn Future<Output = bool> + Send + 'static)>>`
found struct `Pin<Box<dyn Future<Output = ()> + Send>>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
3 changes: 0 additions & 3 deletions src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ fn foo<F: Future<Output=i32> + Send + 'static>(x: F) -> BoxFuture<'static, i32>
x //~ ERROR mismatched types
}

// This case is still subpar:
// `Pin::new(x)`: store this in the heap by calling `Box::new`: `Box::new(x)`
// Should suggest changing the code from `Pin::new` to `Box::pin`.
fn bar<F: Future<Output=i32> + Send + 'static>(x: F) -> BoxFuture<'static, i32> {
Box::new(x) //~ ERROR mismatched types
}
Expand Down
19 changes: 8 additions & 11 deletions src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ LL | Box::pin(x)
| +++++++++ +

error[E0308]: mismatched types
--> $DIR/expected-boxed-future-isnt-pinned.rs:18:5
--> $DIR/expected-boxed-future-isnt-pinned.rs:15:5
|
LL | fn bar<F: Future<Output=i32> + Send + 'static>(x: F) -> BoxFuture<'static, i32> {
| ----------------------- expected `Pin<Box<(dyn Future<Output = i32> + Send + 'static)>>` because of return type
Expand All @@ -27,23 +27,20 @@ LL | Box::new(x)
= help: use `Box::pin`

error[E0308]: mismatched types
--> $DIR/expected-boxed-future-isnt-pinned.rs:22:14
--> $DIR/expected-boxed-future-isnt-pinned.rs:19:14
|
LL | fn baz<F: Future<Output=i32> + Send + 'static>(x: F) -> BoxFuture<'static, i32> {
| - this type parameter
LL | Pin::new(x)
| ^ expected struct `Box`, found type parameter `F`
| -------- ^ expected struct `Box`, found type parameter `F`
| |
| help: use `Box::pin` to pin and box this expression: `Box::pin`
|
= note: expected struct `Box<dyn Future<Output = i32> + Send>`
found type parameter `F`
= note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html
help: store this in the heap by calling `Box::new`
|
LL | Pin::new(Box::new(x))
| +++++++++ +

error[E0277]: `dyn Future<Output = i32> + Send` cannot be unpinned
--> $DIR/expected-boxed-future-isnt-pinned.rs:22:5
--> $DIR/expected-boxed-future-isnt-pinned.rs:19:5
|
LL | Pin::new(x)
| ^^^^^^^^ the trait `Unpin` is not implemented for `dyn Future<Output = i32> + Send`
Expand All @@ -56,7 +53,7 @@ LL | pub const fn new(pointer: P) -> Pin<P> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: `dyn Future<Output = i32> + Send` cannot be unpinned
--> $DIR/expected-boxed-future-isnt-pinned.rs:27:5
--> $DIR/expected-boxed-future-isnt-pinned.rs:24:5
|
LL | Pin::new(Box::new(x))
| ^^^^^^^^ the trait `Unpin` is not implemented for `dyn Future<Output = i32> + Send`
Expand All @@ -69,7 +66,7 @@ LL | pub const fn new(pointer: P) -> Pin<P> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
--> $DIR/expected-boxed-future-isnt-pinned.rs:31:5
--> $DIR/expected-boxed-future-isnt-pinned.rs:28:5
|
LL | fn zap() -> BoxFuture<'static, i32> {
| ----------------------- expected `Pin<Box<(dyn Future<Output = i32> + Send + 'static)>>` because of return type
Expand Down