From 2c31c31bb87092fec3da6eefe6d5a3a836c6c5ba Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Fri, 24 Sep 2021 22:14:06 +0000 Subject: [PATCH 1/5] Fix line length --- compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index da8b863e2dbe6..4d25399a12387 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -341,7 +341,10 @@ 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" + ); } } } From 5c14433c00b29fb2065af0eb664e2040f88b4429 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 30 Sep 2021 01:06:56 +0000 Subject: [PATCH 2/5] Fix incorrect Box::pin suggestion The suggestion checked if Pin> could be coeerced to the expected type, but did not check predicates created by the coercion. We now look for predicates that definitely cannot be satisfied before giving the suggestion. The suggestion is marked MaybeIncorrect because we allow predicates that are still ambiguous and can't be proven. --- compiler/rustc_typeck/src/check/coercion.rs | 24 ++++++++++++++++++- .../src/check/fn_ctxt/suggestions.rs | 18 ++++++++++---- .../ui/suggestions/box-future-wrong-output.rs | 22 +++++++++++++++++ .../box-future-wrong-output.stderr | 14 +++++++++++ 4 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/suggestions/box-future-wrong-output.rs create mode 100644 src/test/ui/suggestions/box-future-wrong-output.stderr diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index 07e542b70b90c..556c5d152df30 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -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, @@ -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); @@ -943,6 +944,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.probe(|_| coerce.coerce(source, target)).is_ok() } + /// Same as `try_coerce()`, but without side-effects and attempts to select + /// all predicates created by the coercion. This is useful for e.g. checking + /// that associated types are correct. + pub fn can_coerce_and_satisfy_predicates(&self, expr_ty: Ty<'tcx>, target: Ty<'tcx>) -> bool { + let source = self.resolve_vars_with_obligations(expr_ty); + 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(|_| { + 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 /// how many dereference steps needed to achieve `expr_ty <: target`. If /// it's not possible, return `None`. diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index 4d25399a12387..62cacdbbce34d 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -370,9 +370,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ 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) { + 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(); + if self.can_coerce_and_satisfy_predicates(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`"); @@ -384,11 +386,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { (expr.span.shrink_to_lo(), "Box::pin(".to_string()), (expr.span.shrink_to_hi(), ")".to_string()), ], - Applicability::MachineApplicable, + Applicability::MaybeIncorrect, ); } } true + } else if self.can_coerce_and_satisfy_predicates(pin_found, expected) { + match found.kind() { + ty::Adt(def, _) if def.is_box() => { + err.help("use `Box::pin`"); + true + } + _ => false, + } } else { false } diff --git a/src/test/ui/suggestions/box-future-wrong-output.rs b/src/test/ui/suggestions/box-future-wrong-output.rs new file mode 100644 index 0000000000000..d49819fcb14cf --- /dev/null +++ b/src/test/ui/suggestions/box-future-wrong-output.rs @@ -0,0 +1,22 @@ +// Issue #72117 +// edition:2018 + +use core::future::Future; +use core::pin::Pin; + +pub type BoxFuture<'a, T> = Pin + Send + 'a>>; + +impl 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 +} diff --git a/src/test/ui/suggestions/box-future-wrong-output.stderr b/src/test/ui/suggestions/box-future-wrong-output.stderr new file mode 100644 index 0000000000000..e0c57af25b3d2 --- /dev/null +++ b/src/test/ui/suggestions/box-future-wrong-output.stderr @@ -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 + Send + 'static)>>` + found struct `Pin + Send>>` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From 485ae9f2c077511f7e2838919c948f9b9b0f0f4f Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 30 Sep 2021 01:22:17 +0000 Subject: [PATCH 3/5] Always check predicates in can_coerce This only changed two tests and I consider both changes an improvement. --- compiler/rustc_typeck/src/check/coercion.rs | 16 +++------------- .../src/check/fn_ctxt/suggestions.rs | 4 ++-- src/test/ui/cross/cross-borrow-trait.stderr | 6 ++---- src/test/ui/dst/dst-bad-coercions.stderr | 12 ++++-------- 4 files changed, 11 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index 556c5d152df30..a87318ff34e6d 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -934,20 +934,10 @@ 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); - - 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() - } - - /// Same as `try_coerce()`, but without side-effects and attempts to select - /// all predicates created by the coercion. This is useful for e.g. checking - /// that associated types are correct. - pub fn can_coerce_and_satisfy_predicates(&self, expr_ty: Ty<'tcx>, target: Ty<'tcx>) -> bool { let source = self.resolve_vars_with_obligations(expr_ty); debug!("coercion::can_with_predicates({:?} -> {:?})", source, target); diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index 62cacdbbce34d..7fe841c381562 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -373,7 +373,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { 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(); - if self.can_coerce_and_satisfy_predicates(pin_box_found, expected) { + 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() => { @@ -391,7 +391,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } true - } else if self.can_coerce_and_satisfy_predicates(pin_found, expected) { + } else if self.can_coerce(pin_found, expected) { match found.kind() { ty::Adt(def, _) if def.is_box() => { err.help("use `Box::pin`"); diff --git a/src/test/ui/cross/cross-borrow-trait.stderr b/src/test/ui/cross/cross-borrow-trait.stderr index f693a3149b2a1..81f309eae087c 100644 --- a/src/test/ui/cross/cross-borrow-trait.stderr +++ b/src/test/ui/cross/cross-borrow-trait.stderr @@ -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` diff --git a/src/test/ui/dst/dst-bad-coercions.stderr b/src/test/ui/dst/dst-bad-coercions.stderr index 3e23c5f5c7443..01f862ed516e9 100644 --- a/src/test/ui/dst/dst-bad-coercions.stderr +++ b/src/test/ui/dst/dst-bad-coercions.stderr @@ -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` @@ -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` From 156c9222f52acb1a842ffc2d6e814fca8937cbf0 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 30 Sep 2021 01:29:24 +0000 Subject: [PATCH 4/5] Move misplaced comment --- .../ui/suggestions/expected-boxed-future-isnt-pinned.rs | 6 +++--- .../ui/suggestions/expected-boxed-future-isnt-pinned.stderr | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs index 5dee0f5dae0b0..89a36e89b0acf 100644 --- a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs @@ -11,13 +11,13 @@ fn foo + 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 + Send + 'static>(x: F) -> BoxFuture<'static, i32> { Box::new(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 baz + Send + 'static>(x: F) -> BoxFuture<'static, i32> { Pin::new(x) //~ ERROR mismatched types //~^ ERROR E0277 diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr index ff08178cb7470..f0af37e0cbe8a 100644 --- a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr @@ -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 + Send + 'static>(x: F) -> BoxFuture<'static, i32> { | ----------------------- expected `Pin + Send + 'static)>>` because of return type From a8558e9efabc6640e8c1b1c353e62c233624f616 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 30 Sep 2021 03:33:12 +0000 Subject: [PATCH 5/5] Exit early if expected type is not an adt --- compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index 7fe841c381562..339c46616a590 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -364,11 +364,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return false; } let pin_did = self.tcx.lang_items().pin_type(); + // This guards the `unwrap` and `mk_box` below. + if pin_did.is_none() || self.tcx.lang_items().owned_box().is_none() { + return false; + } 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, - _ => {} + ty::Adt(def, _) if Some(def.did) == pin_did => (), + _ => return false, } let box_found = self.tcx.mk_box(found); let pin_box_found = self.tcx.mk_lang_item(box_found, LangItem::Pin).unwrap();