From 286503ace2fd1fc8ac8bf8aa10378fb93763d99f Mon Sep 17 00:00:00 2001 From: Michael Hewson Date: Sun, 16 Dec 2018 16:47:37 -0500 Subject: [PATCH] Refactor and add comments to code in receiver_is_valid also updated some error messages removed the code manually checking for `receiver_ty: Deref`, in favour of using autoderef but only doing one iteration. This will cause error messages to be more consistent. Before, a "mismatched method receiver" error would be emitted when `receiver_ty` was valid except for a lifetime parameter, but only when `feature(arbitrary_self_types)` was enabled, and without the feature flag the error would be "uncoercible receiver". Now it emits "mismatched method receiver" in both cases. --- src/librustc_typeck/check/wfcheck.rs | 91 +++++++------------ src/test/ui/span/issue-27522.rs | 2 +- src/test/ui/span/issue-27522.stderr | 4 +- src/test/ui/ufcs/ufcs-explicit-self-bad.rs | 6 +- .../ui/ufcs/ufcs-explicit-self-bad.stderr | 12 +-- 5 files changed, 46 insertions(+), 69 deletions(-) diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs index d35a4dfe0b91d..e74b1ae47e39d 100644 --- a/src/librustc_typeck/check/wfcheck.rs +++ b/src/librustc_typeck/check/wfcheck.rs @@ -760,8 +760,8 @@ fn check_method_receiver<'fcx, 'gcx, 'tcx>(fcx: &FnCtxt<'fcx, 'gcx, 'tcx>, if !receiver_is_valid(fcx, span, receiver_ty, self_ty, true) { // report error, arbitrary_self_types was enabled fcx.tcx.sess.diagnostic().mut_span_err( - span, &format!("invalid `self` type: {:?}", receiver_ty) - ).note(&format!("type must be `{:?}` or a type that dereferences to it", self_ty)) + span, &format!("invalid method receiver type: {:?}", receiver_ty) + ).note("type of `self` must be `Self` or a type that dereferences to it") .help("consider changing to `self`, `&self`, `&mut self`, or `self: Box`") .code(DiagnosticId::Error("E0307".into())) .emit(); @@ -785,8 +785,8 @@ fn check_method_receiver<'fcx, 'gcx, 'tcx>(fcx: &FnCtxt<'fcx, 'gcx, 'tcx>, } else { // report error, would not have worked with arbitrary_self_types fcx.tcx.sess.diagnostic().mut_span_err( - span, &format!("invalid `self` type: {:?}", receiver_ty) - ).note(&format!("type must be `{:?}` or a type that dereferences to it", self_ty)) + span, &format!("invalid method receiver type: {:?}", receiver_ty) + ).note("type must be `Self` or a type that dereferences to it") .help("consider changing to `self`, `&self`, `&mut self`, or `self: Box`") .code(DiagnosticId::Error("E0307".into())) .emit(); @@ -800,9 +800,9 @@ fn check_method_receiver<'fcx, 'gcx, 'tcx>(fcx: &FnCtxt<'fcx, 'gcx, 'tcx>, /// through a `*const/mut T` raw pointer. If the feature is not enabled, the requirements are more /// strict: `receiver_ty` must implement `Receiver` and directly implement `Deref`. /// -/// NB: there are cases this function returns `true` but then causes an error to be raised later, +/// NB: there are cases this function returns `true` but causes an error to be emitted, /// particularly when `receiver_ty` derefs to a type that is the same as `self_ty` but has the -/// wrong lifetime. +/// wrong lifetime. Be careful of this if you are calling this function speculatively. fn receiver_is_valid<'fcx, 'tcx, 'gcx>( fcx: &FnCtxt<'fcx, 'gcx, 'tcx>, span: Span, @@ -814,6 +814,7 @@ fn receiver_is_valid<'fcx, 'tcx, 'gcx>( let can_eq_self = |ty| fcx.infcx.can_eq(fcx.param_env, self_ty, ty).is_ok(); + // `self: Self` is always valid if can_eq_self(receiver_ty) { if let Some(mut err) = fcx.demand_eqtype_with_origin(&cause, self_ty, receiver_ty) { err.emit(); @@ -823,32 +824,47 @@ fn receiver_is_valid<'fcx, 'tcx, 'gcx>( let mut autoderef = fcx.autoderef(span, receiver_ty); + // the `arbitrary_self_types` feature allows raw pointer receivers like `self: *const Self` if arbitrary_self_types_enabled { autoderef = autoderef.include_raw_pointers(); } - // skip the first type, we know its not equal to `self_ty` + // the first type is `receiver_ty`, which we know its not equal to `self_ty`. skip it. autoderef.next(); - let potential_self_ty = loop { + // keep dereferencing `receiver_ty` until we get to `self_ty` + loop { if let Some((potential_self_ty, _)) = autoderef.next() { debug!("receiver_is_valid: potential self type `{:?}` to match `{:?}`", potential_self_ty, self_ty); if can_eq_self(potential_self_ty) { - break potential_self_ty + autoderef.finalize(fcx); + + if let Some(mut err) = fcx.demand_eqtype_with_origin( + &cause, self_ty, potential_self_ty + ) { + err.emit(); + } + + break } } else { debug!("receiver_is_valid: type `{:?}` does not deref to `{:?}`", receiver_ty, self_ty); return false } - }; - if !arbitrary_self_types_enabled { - // check that receiver_ty: Receiver + // without the `arbitrary_self_types` feature, `receiver_ty` must directly deref to + // `self_ty`. Enforce this by only doing one iteration of the loop + if !arbitrary_self_types_enabled { + return false + } + } - let receiver_trait_def_id = match fcx.tcx.lang_items().receiver_trait() { + // without `feature(arbitrary_self_types)`, we require that `receiver_ty` implements `Receiver` + if !arbitrary_self_types_enabled { + let trait_def_id = match fcx.tcx.lang_items().receiver_trait() { Some(did) => did, None => { debug!("receiver_is_valid: missing Receiver trait"); @@ -856,63 +872,24 @@ fn receiver_is_valid<'fcx, 'tcx, 'gcx>( } }; - let receiver_trait_ref = ty::TraitRef{ - def_id: receiver_trait_def_id, + let trait_ref = ty::TraitRef{ + def_id: trait_def_id, substs: fcx.tcx.mk_substs_trait(receiver_ty, &[]), }; - let receiver_obligation = traits::Obligation::new( + let obligation = traits::Obligation::new( cause.clone(), fcx.param_env, - receiver_trait_ref.to_predicate() + trait_ref.to_predicate() ); - if !fcx.predicate_must_hold(&receiver_obligation) { + if !fcx.predicate_must_hold(&obligation) { debug!("receiver_is_valid: type `{:?}` does not implement `Receiver` trait", receiver_ty); return false } - - let deref_trait_def_id = match fcx.tcx.lang_items().deref_trait() { - Some(did) => did, - None => { - debug!("receiver_is_valid: missing Deref trait"); - return false - } - }; - - let deref_trait_ref = ty::TraitRef { - def_id: deref_trait_def_id, - substs: fcx.tcx.mk_substs_trait(receiver_ty, &[]), - }; - - let projection_ty = ty::ProjectionTy::from_ref_and_name( - fcx.tcx, deref_trait_ref, ast::Ident::from_str("Target") - ); - - let projection_predicate = ty::Binder::dummy(ty::ProjectionPredicate { - projection_ty, ty: self_ty - }).to_predicate(); - - let deref_obligation = traits::Obligation::new( - cause.clone(), - fcx.param_env, - projection_predicate, - ); - - if !fcx.predicate_must_hold(&deref_obligation) { - debug!("receiver_is_valid: type `{:?}` does not directly deref to `{:?}`", - receiver_ty, self_ty); - return false - } - } - - if let Some(mut err) = fcx.demand_eqtype_with_origin(&cause, self_ty, potential_self_ty) { - err.emit(); } - autoderef.finalize(fcx); - true } diff --git a/src/test/ui/span/issue-27522.rs b/src/test/ui/span/issue-27522.rs index 1e3eba4bf3631..11c90833e7181 100644 --- a/src/test/ui/span/issue-27522.rs +++ b/src/test/ui/span/issue-27522.rs @@ -13,7 +13,7 @@ struct SomeType {} trait Foo { - fn handler(self: &SomeType); //~ ERROR invalid `self` type + fn handler(self: &SomeType); //~ ERROR invalid method receiver type } fn main() {} diff --git a/src/test/ui/span/issue-27522.stderr b/src/test/ui/span/issue-27522.stderr index 9b61ecae6512e..767b99a92cb2d 100644 --- a/src/test/ui/span/issue-27522.stderr +++ b/src/test/ui/span/issue-27522.stderr @@ -1,7 +1,7 @@ -error[E0307]: invalid `self` type: &SomeType +error[E0307]: invalid method receiver type: &SomeType --> $DIR/issue-27522.rs:16:22 | -LL | fn handler(self: &SomeType); //~ ERROR invalid `self` type +LL | fn handler(self: &SomeType); //~ ERROR invalid method receiver type | ^^^^^^^^^ | = note: type must be `Self` or a type that dereferences to it diff --git a/src/test/ui/ufcs/ufcs-explicit-self-bad.rs b/src/test/ui/ufcs/ufcs-explicit-self-bad.rs index a0d1f2dc3312e..f87541b56acf9 100644 --- a/src/test/ui/ufcs/ufcs-explicit-self-bad.rs +++ b/src/test/ui/ufcs/ufcs-explicit-self-bad.rs @@ -16,7 +16,7 @@ struct Foo { impl Foo { fn foo(self: isize, x: isize) -> isize { - //~^ ERROR invalid `self` type + //~^ ERROR invalid method receiver type self.f + x } } @@ -27,11 +27,11 @@ struct Bar { impl Bar { fn foo(self: Bar, x: isize) -> isize { - //~^ ERROR invalid `self` type + //~^ ERROR invalid method receiver type x } fn bar(self: &Bar, x: isize) -> isize { - //~^ ERROR invalid `self` type + //~^ ERROR invalid method receiver type x } } diff --git a/src/test/ui/ufcs/ufcs-explicit-self-bad.stderr b/src/test/ui/ufcs/ufcs-explicit-self-bad.stderr index fce74605cad38..a229dabcce4a4 100644 --- a/src/test/ui/ufcs/ufcs-explicit-self-bad.stderr +++ b/src/test/ui/ufcs/ufcs-explicit-self-bad.stderr @@ -1,28 +1,28 @@ -error[E0307]: invalid `self` type: isize +error[E0307]: invalid method receiver type: isize --> $DIR/ufcs-explicit-self-bad.rs:18:18 | LL | fn foo(self: isize, x: isize) -> isize { | ^^^^^ | - = note: type must be `Foo` or a type that dereferences to it + = note: type must be `Self` or a type that dereferences to it = help: consider changing to `self`, `&self`, `&mut self`, or `self: Box` -error[E0307]: invalid `self` type: Bar +error[E0307]: invalid method receiver type: Bar --> $DIR/ufcs-explicit-self-bad.rs:29:18 | LL | fn foo(self: Bar, x: isize) -> isize { | ^^^^^^^^^^ | - = note: type must be `Bar` or a type that dereferences to it + = note: type must be `Self` or a type that dereferences to it = help: consider changing to `self`, `&self`, `&mut self`, or `self: Box` -error[E0307]: invalid `self` type: &Bar +error[E0307]: invalid method receiver type: &Bar --> $DIR/ufcs-explicit-self-bad.rs:33:18 | LL | fn bar(self: &Bar, x: isize) -> isize { | ^^^^^^^^^^^ | - = note: type must be `Bar` or a type that dereferences to it + = note: type must be `Self` or a type that dereferences to it = help: consider changing to `self`, `&self`, `&mut self`, or `self: Box` error[E0308]: mismatched method receiver