From b4964f7dcfc2850736cfee10573bfb968460dab4 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 25 Oct 2016 02:04:57 +0200 Subject: [PATCH 01/14] Start of implementation of proposal for E0308 --- src/librustc/infer/error_reporting.rs | 79 ++++++++++++++++++++++++++- src/librustc/ty/sty.rs | 1 + 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/src/librustc/infer/error_reporting.rs b/src/librustc/infer/error_reporting.rs index 47c0bc5fd60c5..2f1c6006f2ae3 100644 --- a/src/librustc/infer/error_reporting.rs +++ b/src/librustc/infer/error_reporting.rs @@ -82,7 +82,7 @@ use hir::def::Def; use hir::def_id::DefId; use infer::{self, TypeOrigin}; use middle::region; -use ty::{self, TyCtxt, TypeFoldable}; +use ty::{self, ImplOrTraitItem, Ty, TyCtxt, TypeFoldable}; use ty::{Region, ReFree}; use ty::error::TypeError; @@ -548,7 +548,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { { let expected_found = match values { None => None, - Some(values) => match self.values_str(&values) { + Some(ref values) => match self.values_str(&values) { Some((expected, found)) => Some((expected, found)), None => { // Derived error. Cancel the emitter. @@ -581,6 +581,27 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { diag.note_expected_found(&"type", &expected, &found); } } + + if let Some((found, (expected_ty, _))) = self.get_ids(values) { + // look for expected with found id + self.tcx.populate_inherent_implementations_for_type_if_necessary(found); + if let Some(impl_infos) = self.tcx.inherent_impls.borrow().get(&found) { + let mut methods = Vec::new(); + for impl_ in impl_infos { + methods.append(&mut self.tcx + .impl_or_trait_items(*impl_) + .iter() + .map(|&did| self.tcx.impl_or_trait_item(did)) + .filter(|x| { + self.matches_return_type(x, &expected_ty) + }) + .collect()); + } + for method in methods { + println!("==> {:?}", method.name()); + } + } + } } diag.span_label(span, &terr); @@ -621,6 +642,60 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } } + fn matches_return_type(&self, method: &ImplOrTraitItem<'tcx>, expected: &ty::Ty<'tcx>) -> bool { + match *method { + ImplOrTraitItem::MethodTraitItem(ref x) => { + self.can_sub_types(x.fty.sig.skip_binder().output, expected).is_ok() + } + _ => false, + } + } + + fn get_id(&self, ty: Ty<'tcx>) -> Option { + match ty.sty { + ty::TyTrait(box ref data) => Some(data.principal.def_id()), + ty::TyAdt(def, _) => Some(def.did), + ty::TyBox(ref ty) => self.get_id(*ty), // since we don't want box's methods by type's + ty::TyChar => self.tcx.lang_items.char_impl(), + ty::TyStr => self.tcx.lang_items.str_impl(), + ty::TySlice(_) => self.tcx.lang_items.slice_impl(), + ty::TyRawPtr(ty::TypeAndMut { ty: _, mutbl: hir::MutImmutable }) => { + self.tcx.lang_items.const_ptr_impl() + } + ty::TyRawPtr(ty::TypeAndMut { ty: _, mutbl: hir::MutMutable }) => { + self.tcx.lang_items.mut_ptr_impl() + } + ty::TyInt(ast::IntTy::I8) => self.tcx.lang_items.i8_impl(), + ty::TyInt(ast::IntTy::I16) => self.tcx.lang_items.i16_impl(), + ty::TyInt(ast::IntTy::I32) => self.tcx.lang_items.i32_impl(), + ty::TyInt(ast::IntTy::I64) => self.tcx.lang_items.i64_impl(), + ty::TyInt(ast::IntTy::Is) => self.tcx.lang_items.isize_impl(), + ty::TyUint(ast::UintTy::U8) => self.tcx.lang_items.u8_impl(), + ty::TyUint(ast::UintTy::U16) => self.tcx.lang_items.u16_impl(), + ty::TyUint(ast::UintTy::U32) => self.tcx.lang_items.u32_impl(), + ty::TyUint(ast::UintTy::U64) => self.tcx.lang_items.u64_impl(), + ty::TyUint(ast::UintTy::Us) => self.tcx.lang_items.usize_impl(), + ty::TyFloat(ast::FloatTy::F32) => self.tcx.lang_items.f32_impl(), + ty::TyFloat(ast::FloatTy::F64) => self.tcx.lang_items.f64_impl(), + _ => None, + } + } + + // Yep, returned value super ugly. it'll certainly become `Option<(DefId, ty::Ty<'tcx>)>` + // in a close future. Or maybe a struct? + fn get_ids(&self, values: Option>) -> Option<(DefId, (ty::Ty<'tcx>, DefId))> { + match values { + // for now, only handling non trait types + Some(infer::Types(ref exp_found)) => { + match (self.get_id(exp_found.found), self.get_id(exp_found.expected)) { + (Some(found), Some(expected)) => Some((found, (exp_found.expected, expected))), + _ => None, + } + } + _ => None, + } + } + fn expected_found_str>( &self, exp_found: &ty::error::ExpectedFound) diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index 92dfb883ef301..29262b283cdb2 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -425,6 +425,7 @@ pub struct ProjectionTy<'tcx> { pub struct BareFnTy<'tcx> { pub unsafety: hir::Unsafety, pub abi: abi::Abi, + /// Signature (inputs and output) of this function type. pub sig: PolyFnSig<'tcx>, } From 05796e238ae485fa8c1e120165b8c5e7bc5ca7d5 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 25 Oct 2016 22:19:19 +0200 Subject: [PATCH 02/14] Add safe_suggestion attribute --- src/libcollections/lib.rs | 1 + src/libcollections/string.rs | 1 + src/librustc/infer/error_reporting.rs | 29 ++++++++++++++++++++++----- src/libsyntax/feature_gate.rs | 8 ++++++++ 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/libcollections/lib.rs b/src/libcollections/lib.rs index 23d6edd6d794e..2abba4b344180 100644 --- a/src/libcollections/lib.rs +++ b/src/libcollections/lib.rs @@ -28,6 +28,7 @@ #![cfg_attr(test, allow(deprecated))] // rand #![cfg_attr(not(stage0), deny(warnings))] +#![cfg_attr(not(stage0), feature(safe_suggestion))] #![feature(alloc)] #![feature(allow_internal_unstable)] diff --git a/src/libcollections/string.rs b/src/libcollections/string.rs index 348eb6fb5ffa4..3712bd4b7ab8d 100644 --- a/src/libcollections/string.rs +++ b/src/libcollections/string.rs @@ -1234,6 +1234,7 @@ impl String { /// assert_eq!(a.len(), 3); /// ``` #[inline] + #[cfg_attr(not(stage0), safe_suggestion)] #[stable(feature = "rust1", since = "1.0.0")] pub fn len(&self) -> usize { self.vec.len() diff --git a/src/librustc/infer/error_reporting.rs b/src/librustc/infer/error_reporting.rs index 2f1c6006f2ae3..fece9b55a7c2a 100644 --- a/src/librustc/infer/error_reporting.rs +++ b/src/librustc/infer/error_reporting.rs @@ -586,19 +586,29 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // look for expected with found id self.tcx.populate_inherent_implementations_for_type_if_necessary(found); if let Some(impl_infos) = self.tcx.inherent_impls.borrow().get(&found) { - let mut methods = Vec::new(); + let mut methods: Vec<(Option, DefId, ImplOrTraitItem<'tcx>)> = Vec::new(); for impl_ in impl_infos { methods.append(&mut self.tcx .impl_or_trait_items(*impl_) .iter() - .map(|&did| self.tcx.impl_or_trait_item(did)) - .filter(|x| { + .map(|&did| (None, did, self.tcx.impl_or_trait_item(did))) + .filter(|&(_, _, ref x)| { self.matches_return_type(x, &expected_ty) }) .collect()); } - for method in methods { - println!("==> {:?}", method.name()); + let safe_suggestions: Vec<_> = methods.iter() + .map(|&(_, ref id, ref x)| (self.find_attr(*id, "safe_suggestion"), id, x)) + .filter(|&(ref res, _, _)| res.is_some()) + .collect(); + if safe_suggestions.len() > 0 { + for (_, _, method) in safe_suggestions { + println!("safe ==> {:?}", method.name()); + } + } else { + for &(_, _, ref method) in methods.iter() { + println!("not safe ==> {:?}", method.name()); + } } } } @@ -614,6 +624,15 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { self.tcx.note_and_explain_type_err(diag, terr, span); } + fn find_attr(&self, def_id: DefId, attr_name: &str) -> Option { + for item in self.tcx.get_attrs(def_id).iter() { + if item.check_name(attr_name) { + return Some(item.clone()); + } + } + None + } + pub fn report_and_explain_type_error(&self, trace: TypeTrace<'tcx>, terr: &TypeError<'tcx>) diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index ba16208715109..d54c72c663fb3 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -315,6 +315,9 @@ declare_features! ( // Allows using `Self` and associated types in struct expressions and patterns. (active, more_struct_aliases, "1.14.0", Some(37544)), + + // Allow safe suggestions for potential type conversions. + (active, safe_suggestion, "1.0.0", Some(37384)), ); declare_features! ( @@ -649,6 +652,11 @@ pub const KNOWN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeGat "internal implementation detail", cfg_fn!(rustc_attrs))), + ("safe_suggestion", Whitelisted, Gated("safe_suggestion", + "the `#[safe_suggestion]` attribute \ + is an experimental feature", + cfg_fn!(safe_suggestion))), + // FIXME: #14408 whitelist docs since rustdoc looks at them ("doc", Whitelisted, Ungated), From 9549fd59af3a6335b92efcde68d0d9b2c699230d Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 26 Oct 2016 23:53:47 +0200 Subject: [PATCH 03/14] Move suggestion list creation to coerce check --- src/librustc/infer/error_reporting.rs | 104 ++++++++++--- src/librustc_typeck/check/demand.rs | 89 ++++++++++- src/librustc_typeck/check/method/mod.rs | 28 +++- src/librustc_typeck/check/method/probe.rs | 171 +++++++++++++++++----- 4 files changed, 329 insertions(+), 63 deletions(-) diff --git a/src/librustc/infer/error_reporting.rs b/src/librustc/infer/error_reporting.rs index fece9b55a7c2a..0fa0c84114793 100644 --- a/src/librustc/infer/error_reporting.rs +++ b/src/librustc/infer/error_reporting.rs @@ -82,13 +82,14 @@ use hir::def::Def; use hir::def_id::DefId; use infer::{self, TypeOrigin}; use middle::region; -use ty::{self, ImplOrTraitItem, Ty, TyCtxt, TypeFoldable}; +use ty::{self, /*ImplOrTraitItem, Ty,*/ TyCtxt, TypeFoldable}; use ty::{Region, ReFree}; use ty::error::TypeError; use std::cell::{Cell, RefCell}; use std::char::from_u32; use std::fmt; +//use std::rc::Rc; use syntax::ast; use syntax::parse::token; use syntax::ptr::P; @@ -232,6 +233,22 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } } +/*struct MethodInfo<'tcx> { + ast: Option, + id: DefId, + item: Rc>, +} + +impl<'tcx> MethodInfo<'tcx> { + fn new(ast: Option, id: DefId, item: Rc>) -> MethodInfo { + MethodInfo { + ast: ast, + id: id, + item: item, + } + } +}*/ + impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { pub fn report_region_errors(&self, errors: &Vec>) { @@ -582,36 +599,53 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } } - if let Some((found, (expected_ty, _))) = self.get_ids(values) { + //if let Some((found, (expected_ty, expected))) = self.get_ids(values) { // look for expected with found id - self.tcx.populate_inherent_implementations_for_type_if_necessary(found); + /*self.tcx.populate_inherent_implementations_for_type_if_necessary(found); if let Some(impl_infos) = self.tcx.inherent_impls.borrow().get(&found) { - let mut methods: Vec<(Option, DefId, ImplOrTraitItem<'tcx>)> = Vec::new(); + let mut methods: Vec = Vec::new(); for impl_ in impl_infos { methods.append(&mut self.tcx .impl_or_trait_items(*impl_) .iter() - .map(|&did| (None, did, self.tcx.impl_or_trait_item(did))) - .filter(|&(_, _, ref x)| { - self.matches_return_type(x, &expected_ty) + .map(|&did| MethodInfo::new(None, did, Rc::new(self.tcx.impl_or_trait_item(did)))) + .filter(|ref x| { + self.matches_return_type(&*x.item, &expected_ty) }) .collect()); } - let safe_suggestions: Vec<_> = methods.iter() - .map(|&(_, ref id, ref x)| (self.find_attr(*id, "safe_suggestion"), id, x)) - .filter(|&(ref res, _, _)| res.is_some()) - .collect(); - if safe_suggestions.len() > 0 { - for (_, _, method) in safe_suggestions { - println!("safe ==> {:?}", method.name()); - } - } else { - for &(_, _, ref method) in methods.iter() { - println!("not safe ==> {:?}", method.name()); + for did in self.tcx.sess.cstore.implementations_of_trait(None) { + if did == found { + methods.append( + self.tcx.sess.cstore.impl_or_trait_items(did) + .iter() + .map(|&did| MethodInfo::new(None, did, Rc::new(self.tcx.impl_or_trait_item(did)))) + .filter(|ref x| { + self.matches_return_type(&*x.item, &expected_ty) + }) + .collect()); + ; } } - } - } + let safe_suggestions: Vec<_> = + methods.iter() + .map(|ref x| MethodInfo::new(self.find_attr(x.id, "safe_suggestion"), x.id, x.item.clone())) + .filter(|ref x| x.ast.is_some()) + .collect(); + if safe_suggestions.len() > 0 { + println!("safe"); + self.get_best_match(&safe_suggestions); + } else { + println!("not safe"); + self.get_best_match(&methods); + }*/ + /*let mode = probe::Mode::MethodCall; + if let Ok(ret) = self.probe_return(DUMMY_SP, mode, expected, found, DUMMY_NODE_ID) { + println!("got it"); + } else { + println!("sad..."); + }*/ + //} } diag.span_label(span, &terr); @@ -624,6 +658,23 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { self.tcx.note_and_explain_type_err(diag, terr, span); } + /*fn get_best_match(&self, methods: &[MethodInfo<'tcx>]) -> String { + let no_argument_methods: Vec<&MethodInfo> = + methods.iter() + .filter(|ref x| self.has_not_input_arg(&*x.item)) + .collect(); + if no_argument_methods.len() > 0 { + for ref method in no_argument_methods { + println!("best match ==> {:?}", method.item.name()); + } + } else { + for ref method in methods.iter() { + println!("not best ==> {:?}", method.item.name()); + } + } + String::new() + } + fn find_attr(&self, def_id: DefId, attr_name: &str) -> Option { for item in self.tcx.get_attrs(def_id).iter() { if item.check_name(attr_name) { @@ -631,7 +682,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } } None - } + }*/ pub fn report_and_explain_type_error(&self, trace: TypeTrace<'tcx>, @@ -661,6 +712,15 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } } + /*fn has_not_input_arg(&self, method: &ImplOrTraitItem<'tcx>) -> bool { + match *method { + ImplOrTraitItem::MethodTraitItem(ref x) => { + x.fty.sig.skip_binder().inputs.len() == 1 + } + _ => false, + } + } + fn matches_return_type(&self, method: &ImplOrTraitItem<'tcx>, expected: &ty::Ty<'tcx>) -> bool { match *method { ImplOrTraitItem::MethodTraitItem(ref x) => { @@ -713,7 +773,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } _ => None, } - } + }*/ fn expected_found_str>( &self, diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index d622bc7f751d7..08c7a22e880f6 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -13,8 +13,32 @@ use check::FnCtxt; use rustc::ty::Ty; use rustc::infer::{InferOk, TypeOrigin}; -use syntax_pos::Span; +use syntax::ast; +use syntax_pos::{self, Span}; use rustc::hir; +use rustc::ty::{self, ImplOrTraitItem}; + +use hir::def_id::DefId; + +use std::rc::Rc; + +use super::method::probe; + +struct MethodInfo<'tcx> { + ast: Option, + id: DefId, + item: Rc>, +} + +impl<'tcx> MethodInfo<'tcx> { + fn new(ast: Option, id: DefId, item: Rc>) -> MethodInfo { + MethodInfo { + ast: ast, + id: id, + item: item, + } + } +} impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // Requires that the two types unify, and prints an error message if @@ -58,7 +82,70 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Err(e) = self.try_coerce(expr, checked_ty, expected) { let origin = TypeOrigin::Misc(expr.span); let expr_ty = self.resolve_type_vars_with_obligations(checked_ty); + let mode = probe::Mode::MethodCall; + if let Ok(methods) = self.probe_return(syntax_pos::DUMMY_SP, mode, expected, + checked_ty, ast::DUMMY_NODE_ID) { + let suggestions: Vec<_> = + methods.iter() + .filter_map(|ref x| { + if let Some(id) = self.get_impl_id(&x.item) { + Some(MethodInfo::new(None, id, Rc::new(x.item.clone()))) + } else { + None + }}) + .collect(); + let safe_suggestions: Vec<_> = + suggestions.iter() + .map(|ref x| MethodInfo::new( + self.find_attr(x.id, "safe_suggestion"), + x.id, + x.item.clone())) + .filter(|ref x| x.ast.is_some()) + .collect(); + if safe_suggestions.len() > 0 { + self.get_best_match(&safe_suggestions); + } else { + self.get_best_match(&suggestions); + } + } self.report_mismatched_types(origin, expected, expr_ty, e); } } + + fn get_best_match(&self, methods: &[MethodInfo<'tcx>]) -> String { + if methods.len() == 1 { + println!("unique match ==> {:?}", methods[0].item.name()); + return String::new(); + } + let no_argument_methods: Vec<&MethodInfo> = + methods.iter() + .filter(|ref x| self.has_not_input_arg(&*x.item)) + .collect(); + if no_argument_methods.len() > 0 { + for ref method in no_argument_methods { + println!("best match ==> {:?}", method.item.name()); + } + } else { + for ref method in methods.iter() { + println!("not best ==> {:?}", method.item.name()); + } + } + String::new() + } + + fn get_impl_id(&self, impl_: &ImplOrTraitItem<'tcx>) -> Option { + match *impl_ { + ty::ImplOrTraitItem::MethodTraitItem(ref m) => Some((*m).def_id), + _ => None, + } + } + + fn has_not_input_arg(&self, method: &ImplOrTraitItem<'tcx>) -> bool { + match *method { + ImplOrTraitItem::MethodTraitItem(ref x) => { + x.fty.sig.skip_binder().inputs.len() == 1 + } + _ => false, + } + } } diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index 2df562f9ade46..e30aa0ac031d1 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -30,7 +30,7 @@ pub use self::CandidateSource::*; pub use self::suggest::AllTraitsVec; mod confirm; -mod probe; +pub mod probe; mod suggest; pub enum MethodError<'tcx> { @@ -130,7 +130,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let mode = probe::Mode::MethodCall; let self_ty = self.resolve_type_vars_if_possible(&self_ty); - let pick = self.probe_method(span, mode, method_name, self_ty, call_expr.id)?; + let pick = self.probe_method(span, mode, method_name, self_ty, call_expr.id)?.remove(0); if let Some(import_id) = pick.import_id { self.tcx.used_trait_imports.borrow_mut().insert(import_id); @@ -353,7 +353,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { expr_id: ast::NodeId) -> Result> { let mode = probe::Mode::Path; - let pick = self.probe_method(span, mode, method_name, self_ty, expr_id)?; + let picks = self.probe_method(span, mode, method_name, self_ty, expr_id)?; + let pick = &picks[0]; if let Some(import_id) = pick.import_id { self.tcx.used_trait_imports.borrow_mut().insert(import_id); @@ -381,4 +382,25 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { .map(|&did| self.tcx.impl_or_trait_item(did)) .find(|m| m.name() == item_name) } + + fn matches_return_type(&self, method: &ty::ImplOrTraitItem<'tcx>, + expected: ty::Ty<'tcx>) -> bool { + match *method { + ty::ImplOrTraitItem::MethodTraitItem(ref x) => { + self.can_sub_types(x.fty.sig.skip_binder().output, expected).is_ok() + } + _ => false, + } + } + + pub fn impl_or_return_item(&self, + def_id: DefId, + return_type: ty::Ty<'tcx>) + -> Option> { + self.tcx + .impl_or_trait_items(def_id) + .iter() + .map(|&did| self.tcx.impl_or_trait_item(did)) + .find(|m| self.matches_return_type(m, return_type)) + } } diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 43837de2f345d..a6566b09197d2 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -31,11 +31,16 @@ use std::rc::Rc; use self::CandidateKind::*; pub use self::PickKind::*; +pub enum LookingFor<'tcx> { + MethodName(ast::Name), + ReturnType(Ty<'tcx>), +} + struct ProbeContext<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { fcx: &'a FnCtxt<'a, 'gcx, 'tcx>, span: Span, mode: Mode, - item_name: ast::Name, + looking_for: LookingFor<'tcx>, steps: Rc>>, opt_simplified_steps: Option>, inherent_candidates: Vec>, @@ -128,7 +133,7 @@ pub enum PickKind<'tcx> { ty::PolyTraitRef<'tcx>), } -pub type PickResult<'tcx> = Result, MethodError<'tcx>>; +pub type PickResult<'tcx> = Result>, MethodError<'tcx>>; #[derive(PartialEq, Eq, Copy, Clone, Debug)] pub enum Mode { @@ -143,6 +148,20 @@ pub enum Mode { } impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { + pub fn probe_return(&self, + span: Span, + mode: Mode, + return_type: Ty<'tcx>, + self_ty: Ty<'tcx>, + scope_expr_id: ast::NodeId) + -> PickResult<'tcx> { + debug!("probe(self_ty={:?}, return_type={}, scope_expr_id={})", + self_ty, + return_type, + scope_expr_id); + self._probe(span, mode, LookingFor::ReturnType(return_type), self_ty, scope_expr_id) + } + pub fn probe_method(&self, span: Span, mode: Mode, @@ -154,6 +173,16 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self_ty, item_name, scope_expr_id); + self._probe(span, mode, LookingFor::MethodName(item_name), self_ty, scope_expr_id) + } + + fn _probe(&self, + span: Span, + mode: Mode, + looking_for: LookingFor<'tcx>, + self_ty: Ty<'tcx>, + scope_expr_id: ast::NodeId) + -> PickResult<'tcx> { // FIXME(#18741) -- right now, creating the steps involves evaluating the // `*` operator, which registers obligations that then escape into @@ -206,14 +235,18 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // that we create during the probe process are removed later self.probe(|_| { let mut probe_cx = - ProbeContext::new(self, span, mode, item_name, steps, opt_simplified_steps); + ProbeContext::new(self, span, mode, looking_for, + steps, opt_simplified_steps); probe_cx.assemble_inherent_candidates(); probe_cx.assemble_extension_candidates_for_traits_in_scope(scope_expr_id)?; probe_cx.pick() }) } - fn create_steps(&self, span: Span, self_ty: Ty<'tcx>) -> Option>> { + fn create_steps(&self, + span: Span, + self_ty: Ty<'tcx>) + -> Option>> { // FIXME: we don't need to create the entire steps in one pass let mut autoderef = self.autoderef(span, self_ty); @@ -246,13 +279,22 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { Some(steps) } + + pub fn find_attr(&self, def_id: DefId, attr_name: &str) -> Option { + for item in self.tcx.get_attrs(def_id).iter() { + if item.check_name(attr_name) { + return Some(item.clone()); + } + } + None + } } impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { fn new(fcx: &'a FnCtxt<'a, 'gcx, 'tcx>, span: Span, mode: Mode, - item_name: ast::Name, + looking_for: LookingFor<'tcx>, steps: Vec>, opt_simplified_steps: Option>) -> ProbeContext<'a, 'gcx, 'tcx> { @@ -260,7 +302,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { fcx: fcx, span: span, mode: mode, - item_name: item_name, + looking_for: looking_for, inherent_candidates: Vec::new(), extension_candidates: Vec::new(), impl_dups: FnvHashSet(), @@ -594,6 +636,16 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { Ok(()) } + fn matches_return_type(&self, method: &ty::ImplOrTraitItem<'tcx>, + expected: &ty::Ty<'tcx>) -> bool { + match *method { + ty::ImplOrTraitItem::MethodTraitItem(ref x) => { + self.can_sub_types(x.fty.sig.skip_binder().output, expected).is_ok() + } + _ => false, + } + } + fn assemble_extension_candidates_for_trait(&mut self, trait_def_id: DefId) -> Result<(), MethodError<'tcx>> { @@ -602,8 +654,18 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { // Check whether `trait_def_id` defines a method with suitable name: let trait_items = self.tcx.trait_items(trait_def_id); - let maybe_item = trait_items.iter() - .find(|item| item.name() == self.item_name); + let maybe_item = match self.looking_for { + LookingFor::MethodName(item_name) => { + trait_items.iter() + .find(|item| item.name() == item_name) + } + LookingFor::ReturnType(item_ty) => { + trait_items.iter() + .find(|item| { + self.matches_return_type(item, &item_ty) + }) + } + }; let item = match maybe_item { Some(i) => i, None => { @@ -848,8 +910,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { // THE ACTUAL SEARCH fn pick(mut self) -> PickResult<'tcx> { - if let Some(r) = self.pick_core() { - return r; + if let Some(ret) = self.pick_core() { + return ret; } let static_candidates = mem::replace(&mut self.static_candidates, vec![]); @@ -865,7 +927,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { self.assemble_extension_candidates_for_all_traits()?; let out_of_scope_traits = match self.pick_core() { - Some(Ok(p)) => vec![p.item.container().id()], + Some(Ok(p)) => p.iter().map(|p| p.item.container().id()).collect(), Some(Err(MethodError::Ambiguity(v))) => { v.into_iter() .map(|source| { @@ -907,9 +969,19 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { fn pick_core(&mut self) -> Option> { let steps = self.steps.clone(); + let mut ret = Vec::new(); - // find the first step that works - steps.iter().filter_map(|step| self.pick_step(step)).next() + for step in steps.iter() { + match self.pick_step(step) { + Some(Ok(mut elems)) => ret.append(&mut elems), + _ => {} + } + } + if ret.len() < 1 { + None + } else { + Some(Ok(ret)) + } } fn pick_step(&mut self, step: &CandidateStep<'tcx>) -> Option> { @@ -940,16 +1012,18 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { } self.pick_method(step.self_ty).map(|r| { - r.map(|mut pick| { - pick.autoderefs = step.autoderefs; - - // Insert a `&*` or `&mut *` if this is a reference type: - if let ty::TyRef(_, mt) = step.self_ty.sty { - pick.autoderefs += 1; - pick.autoref = Some(mt.mutbl); + r.map(|mut picks| { + for pick in picks.iter_mut() { + pick.autoderefs = step.autoderefs; + + // Insert a `&*` or `&mut *` if this is a reference type: + if let ty::TyRef(_, mt) = step.self_ty.sty { + pick.autoderefs += 1; + pick.autoref = Some(mt.mutbl); + } } - pick + picks }) }) } @@ -960,9 +1034,10 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { // In general, during probing we erase regions. See // `impl_self_ty()` for an explanation. let region = tcx.mk_region(ty::ReErased); + let mut res = Vec::new(); // Search through mutabilities in order to find one where pick works: - [hir::MutImmutable, hir::MutMutable] + for _ in [hir::MutImmutable, hir::MutMutable] .iter() .filter_map(|&m| { let autoref_ty = tcx.mk_ref(region, @@ -971,19 +1046,26 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { mutbl: m, }); self.pick_method(autoref_ty).map(|r| { - r.map(|mut pick| { - pick.autoderefs = step.autoderefs; - pick.autoref = Some(m); - pick.unsize = if step.unsize { - Some(step.self_ty) - } else { - None - }; - pick + r.map(|mut picks| { + for pick in picks.iter_mut() { + pick.autoderefs = step.autoderefs; + pick.autoref = Some(m); + pick.unsize = if step.unsize { + Some(step.self_ty) + } else { + None + }; + } + res.append(&mut picks); }) }) - }) - .nth(0) + }) {} + + if res.len() < 1 { + None + } else { + Some(Ok(res)) + } } fn pick_method(&mut self, self_ty: Ty<'tcx>) -> Option> { @@ -1013,7 +1095,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { probes: &[Candidate<'tcx>], possibly_unsatisfied_predicates: &mut Vec>) -> Option> { - let mut applicable_candidates: Vec<_> = probes.iter() + let applicable_candidates: Vec<_> = probes.iter() .filter(|&probe| self.consider_probe(self_ty, probe, possibly_unsatisfied_predicates)) .collect(); @@ -1022,7 +1104,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { if applicable_candidates.len() > 1 { match self.collapse_candidates_to_trait_pick(&applicable_candidates[..]) { Some(pick) => { - return Some(Ok(pick)); + return Some(Ok(vec![pick])); } None => {} } @@ -1033,7 +1115,15 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { return Some(Err(MethodError::Ambiguity(sources))); } - applicable_candidates.pop().map(|probe| Ok(probe.to_unadjusted_pick())) + let ret: Vec<_> = applicable_candidates.iter() + .map(|probe| probe.to_unadjusted_pick()) + .collect(); + + if ret.len() < 1 { + None + } else { + Some(Ok(ret)) + } } fn consider_probe(&self, @@ -1284,7 +1374,14 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { /// Find item with name `item_name` defined in impl/trait `def_id` /// and return it, or `None`, if no such item was defined there. fn impl_or_trait_item(&self, def_id: DefId) -> Option> { - self.fcx.impl_or_trait_item(def_id, self.item_name) + match self.looking_for { + LookingFor::MethodName(name) => { + self.fcx.impl_or_trait_item(def_id, name) + } + LookingFor::ReturnType(return_ty) => { + self.fcx.impl_or_return_item(def_id, return_ty) + } + } } } From 46586886890686065ee73d234c9539907c8c9a0e Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 27 Oct 2016 00:43:00 +0200 Subject: [PATCH 04/14] Return DiagnosticBuilder to add help suggestions --- src/librustc/infer/error_reporting.rs | 156 +------------------------- src/librustc/infer/mod.rs | 4 +- src/librustc_typeck/check/_match.rs | 2 +- src/librustc_typeck/check/demand.rs | 61 ++++++---- src/librustc_typeck/check/mod.rs | 4 +- src/librustc_typeck/coherence/mod.rs | 2 +- src/librustc_typeck/lib.rs | 2 +- 7 files changed, 45 insertions(+), 186 deletions(-) diff --git a/src/librustc/infer/error_reporting.rs b/src/librustc/infer/error_reporting.rs index 0fa0c84114793..c529c401914ca 100644 --- a/src/librustc/infer/error_reporting.rs +++ b/src/librustc/infer/error_reporting.rs @@ -82,14 +82,13 @@ use hir::def::Def; use hir::def_id::DefId; use infer::{self, TypeOrigin}; use middle::region; -use ty::{self, /*ImplOrTraitItem, Ty,*/ TyCtxt, TypeFoldable}; +use ty::{self, TyCtxt, TypeFoldable}; use ty::{Region, ReFree}; use ty::error::TypeError; use std::cell::{Cell, RefCell}; use std::char::from_u32; use std::fmt; -//use std::rc::Rc; use syntax::ast; use syntax::parse::token; use syntax::ptr::P; @@ -233,22 +232,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } } -/*struct MethodInfo<'tcx> { - ast: Option, - id: DefId, - item: Rc>, -} - -impl<'tcx> MethodInfo<'tcx> { - fn new(ast: Option, id: DefId, item: Rc>) -> MethodInfo { - MethodInfo { - ast: ast, - id: id, - item: item, - } - } -}*/ - impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { pub fn report_region_errors(&self, errors: &Vec>) { @@ -598,54 +581,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { diag.note_expected_found(&"type", &expected, &found); } } - - //if let Some((found, (expected_ty, expected))) = self.get_ids(values) { - // look for expected with found id - /*self.tcx.populate_inherent_implementations_for_type_if_necessary(found); - if let Some(impl_infos) = self.tcx.inherent_impls.borrow().get(&found) { - let mut methods: Vec = Vec::new(); - for impl_ in impl_infos { - methods.append(&mut self.tcx - .impl_or_trait_items(*impl_) - .iter() - .map(|&did| MethodInfo::new(None, did, Rc::new(self.tcx.impl_or_trait_item(did)))) - .filter(|ref x| { - self.matches_return_type(&*x.item, &expected_ty) - }) - .collect()); - } - for did in self.tcx.sess.cstore.implementations_of_trait(None) { - if did == found { - methods.append( - self.tcx.sess.cstore.impl_or_trait_items(did) - .iter() - .map(|&did| MethodInfo::new(None, did, Rc::new(self.tcx.impl_or_trait_item(did)))) - .filter(|ref x| { - self.matches_return_type(&*x.item, &expected_ty) - }) - .collect()); - ; - } - } - let safe_suggestions: Vec<_> = - methods.iter() - .map(|ref x| MethodInfo::new(self.find_attr(x.id, "safe_suggestion"), x.id, x.item.clone())) - .filter(|ref x| x.ast.is_some()) - .collect(); - if safe_suggestions.len() > 0 { - println!("safe"); - self.get_best_match(&safe_suggestions); - } else { - println!("not safe"); - self.get_best_match(&methods); - }*/ - /*let mode = probe::Mode::MethodCall; - if let Ok(ret) = self.probe_return(DUMMY_SP, mode, expected, found, DUMMY_NODE_ID) { - println!("got it"); - } else { - println!("sad..."); - }*/ - //} } diag.span_label(span, &terr); @@ -658,32 +593,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { self.tcx.note_and_explain_type_err(diag, terr, span); } - /*fn get_best_match(&self, methods: &[MethodInfo<'tcx>]) -> String { - let no_argument_methods: Vec<&MethodInfo> = - methods.iter() - .filter(|ref x| self.has_not_input_arg(&*x.item)) - .collect(); - if no_argument_methods.len() > 0 { - for ref method in no_argument_methods { - println!("best match ==> {:?}", method.item.name()); - } - } else { - for ref method in methods.iter() { - println!("not best ==> {:?}", method.item.name()); - } - } - String::new() - } - - fn find_attr(&self, def_id: DefId, attr_name: &str) -> Option { - for item in self.tcx.get_attrs(def_id).iter() { - if item.check_name(attr_name) { - return Some(item.clone()); - } - } - None - }*/ - pub fn report_and_explain_type_error(&self, trace: TypeTrace<'tcx>, terr: &TypeError<'tcx>) @@ -712,69 +621,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } } - /*fn has_not_input_arg(&self, method: &ImplOrTraitItem<'tcx>) -> bool { - match *method { - ImplOrTraitItem::MethodTraitItem(ref x) => { - x.fty.sig.skip_binder().inputs.len() == 1 - } - _ => false, - } - } - - fn matches_return_type(&self, method: &ImplOrTraitItem<'tcx>, expected: &ty::Ty<'tcx>) -> bool { - match *method { - ImplOrTraitItem::MethodTraitItem(ref x) => { - self.can_sub_types(x.fty.sig.skip_binder().output, expected).is_ok() - } - _ => false, - } - } - - fn get_id(&self, ty: Ty<'tcx>) -> Option { - match ty.sty { - ty::TyTrait(box ref data) => Some(data.principal.def_id()), - ty::TyAdt(def, _) => Some(def.did), - ty::TyBox(ref ty) => self.get_id(*ty), // since we don't want box's methods by type's - ty::TyChar => self.tcx.lang_items.char_impl(), - ty::TyStr => self.tcx.lang_items.str_impl(), - ty::TySlice(_) => self.tcx.lang_items.slice_impl(), - ty::TyRawPtr(ty::TypeAndMut { ty: _, mutbl: hir::MutImmutable }) => { - self.tcx.lang_items.const_ptr_impl() - } - ty::TyRawPtr(ty::TypeAndMut { ty: _, mutbl: hir::MutMutable }) => { - self.tcx.lang_items.mut_ptr_impl() - } - ty::TyInt(ast::IntTy::I8) => self.tcx.lang_items.i8_impl(), - ty::TyInt(ast::IntTy::I16) => self.tcx.lang_items.i16_impl(), - ty::TyInt(ast::IntTy::I32) => self.tcx.lang_items.i32_impl(), - ty::TyInt(ast::IntTy::I64) => self.tcx.lang_items.i64_impl(), - ty::TyInt(ast::IntTy::Is) => self.tcx.lang_items.isize_impl(), - ty::TyUint(ast::UintTy::U8) => self.tcx.lang_items.u8_impl(), - ty::TyUint(ast::UintTy::U16) => self.tcx.lang_items.u16_impl(), - ty::TyUint(ast::UintTy::U32) => self.tcx.lang_items.u32_impl(), - ty::TyUint(ast::UintTy::U64) => self.tcx.lang_items.u64_impl(), - ty::TyUint(ast::UintTy::Us) => self.tcx.lang_items.usize_impl(), - ty::TyFloat(ast::FloatTy::F32) => self.tcx.lang_items.f32_impl(), - ty::TyFloat(ast::FloatTy::F64) => self.tcx.lang_items.f64_impl(), - _ => None, - } - } - - // Yep, returned value super ugly. it'll certainly become `Option<(DefId, ty::Ty<'tcx>)>` - // in a close future. Or maybe a struct? - fn get_ids(&self, values: Option>) -> Option<(DefId, (ty::Ty<'tcx>, DefId))> { - match values { - // for now, only handling non trait types - Some(infer::Types(ref exp_found)) => { - match (self.get_id(exp_found.found), self.get_id(exp_found.expected)) { - (Some(found), Some(expected)) => Some((found, (exp_found.expected, expected))), - _ => None, - } - } - _ => None, - } - }*/ - fn expected_found_str>( &self, exp_found: &ty::error::ExpectedFound) diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs index 21820ca071921..ced78a5784182 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -1493,7 +1493,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { origin: TypeOrigin, expected: Ty<'tcx>, actual: Ty<'tcx>, - err: TypeError<'tcx>) { + err: TypeError<'tcx>) -> DiagnosticBuilder<'tcx> { let trace = TypeTrace { origin: origin, values: Types(ExpectedFound { @@ -1501,7 +1501,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { found: actual }) }; - self.report_and_explain_type_error(trace, &err).emit(); + self.report_and_explain_type_error(trace, &err) } pub fn report_conflicting_default_types(&self, diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index c842514227ca0..357c41f86c4a5 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -470,7 +470,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } else { (result_ty, arm_ty) }; - self.report_mismatched_types(origin, expected, found, e); + self.report_mismatched_types(origin, expected, found, e).emit(); self.tcx.types.err } }; diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 08c7a22e880f6..645bd31e1808f 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -51,7 +51,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { assert!(obligations.is_empty()); }, Err(e) => { - self.report_mismatched_types(origin, expected, actual, e); + self.report_mismatched_types(origin, expected, actual, e).emit(); } } } @@ -71,7 +71,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { assert!(obligations.is_empty()); }, Err(e) => { - self.report_mismatched_types(origin, expected, actual, e); + self.report_mismatched_types(origin, expected, actual, e).emit(); } } } @@ -83,7 +83,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let origin = TypeOrigin::Misc(expr.span); let expr_ty = self.resolve_type_vars_with_obligations(checked_ty); let mode = probe::Mode::MethodCall; - if let Ok(methods) = self.probe_return(syntax_pos::DUMMY_SP, mode, expected, + let suggestions = + if let Ok(methods) = self.probe_return(syntax_pos::DUMMY_SP, mode, expected, checked_ty, ast::DUMMY_NODE_ID) { let suggestions: Vec<_> = methods.iter() @@ -94,43 +95,55 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { None }}) .collect(); - let safe_suggestions: Vec<_> = - suggestions.iter() - .map(|ref x| MethodInfo::new( - self.find_attr(x.id, "safe_suggestion"), - x.id, - x.item.clone())) - .filter(|ref x| x.ast.is_some()) - .collect(); - if safe_suggestions.len() > 0 { - self.get_best_match(&safe_suggestions); + if suggestions.len() > 0 { + let safe_suggestions: Vec<_> = + suggestions.iter() + .map(|ref x| MethodInfo::new( + self.find_attr(x.id, "safe_suggestion"), + x.id, + x.item.clone())) + .filter(|ref x| x.ast.is_some()) + .collect(); + Some(if safe_suggestions.len() > 0 { + self.get_best_match(&safe_suggestions) + } else { + format!("no safe suggestion found, here are functions which match your \ + needs but be careful:\n - {}", + self.get_best_match(&suggestions)) + }) } else { - self.get_best_match(&suggestions); + None } + } else { + None + }; + let mut err = self.report_mismatched_types(origin, expected, expr_ty, e); + if let Some(suggestions) = suggestions { + err.help(&suggestions); } - self.report_mismatched_types(origin, expected, expr_ty, e); + err.emit(); } } fn get_best_match(&self, methods: &[MethodInfo<'tcx>]) -> String { if methods.len() == 1 { - println!("unique match ==> {:?}", methods[0].item.name()); - return String::new(); + return format!(" - {}", methods[0].item.name()); } let no_argument_methods: Vec<&MethodInfo> = methods.iter() .filter(|ref x| self.has_not_input_arg(&*x.item)) .collect(); if no_argument_methods.len() > 0 { - for ref method in no_argument_methods { - println!("best match ==> {:?}", method.item.name()); - } + no_argument_methods.iter() + .map(|method| format!("{}", method.item.name())) + .collect::>() + .join("\n - ") } else { - for ref method in methods.iter() { - println!("not best ==> {:?}", method.item.name()); - } + methods.iter() + .map(|method| format!("{}", method.item.name())) + .collect::>() + .join("\n - ") } - String::new() } fn get_impl_id(&self, impl_: &ImplOrTraitItem<'tcx>) -> Option { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index d8314bd6c2aed..077967e6095dd 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -2893,7 +2893,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } } Err(e) => { - self.report_mismatched_types(origin, expected, found, e); + self.report_mismatched_types(origin, expected, found, e).emit(); self.tcx.types.err } } @@ -3663,7 +3663,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { match result { Ok(ty) => unified = ty, Err(e) => { - self.report_mismatched_types(origin, unified, e_ty, e); + self.report_mismatched_types(origin, unified, e_ty, e).emit(); } } } diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 4a4dea5b514e5..294bdfc759619 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -374,7 +374,7 @@ impl<'a, 'gcx, 'tcx> CoherenceChecker<'a, 'gcx, 'tcx> { infcx.report_mismatched_types(origin, mk_ptr(mt_b.ty), target, - ty::error::TypeError::Mutability); + ty::error::TypeError::Mutability).emit(); } (mt_a.ty, mt_b.ty, unsize_trait, None) }; diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index 75f0dac59501d..d950ef9d3b988 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -205,7 +205,7 @@ fn require_same_types<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, true } Err(err) => { - infcx.report_mismatched_types(origin, expected, actual, err); + infcx.report_mismatched_types(origin, expected, actual, err).emit(); false } } From c87aa8ebd2354f0260248f26228b21a1940ea2f2 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 27 Oct 2016 20:53:03 +0200 Subject: [PATCH 05/14] Move matches return type method --- src/librustc_typeck/check/method/mod.rs | 2 +- src/librustc_typeck/check/method/probe.rs | 12 +----------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index e30aa0ac031d1..d85baf606391c 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -383,7 +383,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { .find(|m| m.name() == item_name) } - fn matches_return_type(&self, method: &ty::ImplOrTraitItem<'tcx>, + pub fn matches_return_type(&self, method: &ty::ImplOrTraitItem<'tcx>, expected: ty::Ty<'tcx>) -> bool { match *method { ty::ImplOrTraitItem::MethodTraitItem(ref x) => { diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index a6566b09197d2..391c8dfb46c33 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -636,16 +636,6 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { Ok(()) } - fn matches_return_type(&self, method: &ty::ImplOrTraitItem<'tcx>, - expected: &ty::Ty<'tcx>) -> bool { - match *method { - ty::ImplOrTraitItem::MethodTraitItem(ref x) => { - self.can_sub_types(x.fty.sig.skip_binder().output, expected).is_ok() - } - _ => false, - } - } - fn assemble_extension_candidates_for_trait(&mut self, trait_def_id: DefId) -> Result<(), MethodError<'tcx>> { @@ -662,7 +652,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { LookingFor::ReturnType(item_ty) => { trait_items.iter() .find(|item| { - self.matches_return_type(item, &item_ty) + self.fcx.matches_return_type(item, &item_ty) }) } }; From 19bb4ed0702b0a0c26f8d7720aaeaf11a15729b1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 28 Oct 2016 01:04:16 +0200 Subject: [PATCH 06/14] Fix coercion ICE --- src/librustc_typeck/check/method/mod.rs | 21 ------------ src/librustc_typeck/check/method/probe.rs | 40 +++++++++++++---------- src/libsyntax/feature_gate.rs | 3 +- 3 files changed, 24 insertions(+), 40 deletions(-) diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index d85baf606391c..cddd097b3f6d5 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -382,25 +382,4 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { .map(|&did| self.tcx.impl_or_trait_item(did)) .find(|m| m.name() == item_name) } - - pub fn matches_return_type(&self, method: &ty::ImplOrTraitItem<'tcx>, - expected: ty::Ty<'tcx>) -> bool { - match *method { - ty::ImplOrTraitItem::MethodTraitItem(ref x) => { - self.can_sub_types(x.fty.sig.skip_binder().output, expected).is_ok() - } - _ => false, - } - } - - pub fn impl_or_return_item(&self, - def_id: DefId, - return_type: ty::Ty<'tcx>) - -> Option> { - self.tcx - .impl_or_trait_items(def_id) - .iter() - .map(|&did| self.tcx.impl_or_trait_item(did)) - .find(|m| self.matches_return_type(m, return_type)) - } } diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 391c8dfb46c33..e6f9a176ed4e8 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -19,7 +19,7 @@ use hir::def::Def; use rustc::ty::subst::{Subst, Substs}; use rustc::traits; use rustc::ty::{self, Ty, ToPolyTraitRef, TraitRef, TypeFoldable}; -use rustc::infer::{InferOk, TypeOrigin}; +use rustc::infer::{self, InferOk, TypeOrigin}; use rustc::util::nodemap::FnvHashSet; use syntax::ast; use syntax_pos::{Span, DUMMY_SP}; @@ -636,27 +636,27 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { Ok(()) } + pub fn matches_return_type(&self, method: &ty::ImplOrTraitItem<'tcx>, + expected: ty::Ty<'tcx>) -> bool { + match *method { + ty::ImplOrTraitItem::MethodTraitItem(ref x) => { + self.probe(|_| { + let output = self.replace_late_bound_regions_with_fresh_var( + self.span, infer::FnCall, &x.fty.sig.output()); + self.can_sub_types(output.0, expected).is_ok() + }) + } + _ => false, + } + } + fn assemble_extension_candidates_for_trait(&mut self, trait_def_id: DefId) -> Result<(), MethodError<'tcx>> { debug!("assemble_extension_candidates_for_trait(trait_def_id={:?})", trait_def_id); - // Check whether `trait_def_id` defines a method with suitable name: - let trait_items = self.tcx.trait_items(trait_def_id); - let maybe_item = match self.looking_for { - LookingFor::MethodName(item_name) => { - trait_items.iter() - .find(|item| item.name() == item_name) - } - LookingFor::ReturnType(item_ty) => { - trait_items.iter() - .find(|item| { - self.fcx.matches_return_type(item, &item_ty) - }) - } - }; - let item = match maybe_item { + let item = match self.impl_or_trait_item(trait_def_id) { Some(i) => i, None => { return Ok(()); @@ -664,7 +664,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { }; // Check whether `trait_def_id` defines a method with suitable name: - if !self.has_applicable_self(item) { + if !self.has_applicable_self(&item) { debug!("method has inapplicable self"); self.record_static_candidate(TraitSource(trait_def_id)); return Ok(()); @@ -1369,7 +1369,11 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { self.fcx.impl_or_trait_item(def_id, name) } LookingFor::ReturnType(return_ty) => { - self.fcx.impl_or_return_item(def_id, return_ty) + self.tcx + .impl_or_trait_items(def_id) + .iter() + .map(|&did| self.tcx.impl_or_trait_item(did)) + .find(|m| self.matches_return_type(m, return_ty)) } } } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index d54c72c663fb3..cae1db80a99f6 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -652,7 +652,8 @@ pub const KNOWN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeGat "internal implementation detail", cfg_fn!(rustc_attrs))), - ("safe_suggestion", Whitelisted, Gated("safe_suggestion", + ("safe_suggestion", Whitelisted, Gated(Stability::Unstable, + "safe_suggestion", "the `#[safe_suggestion]` attribute \ is an experimental feature", cfg_fn!(safe_suggestion))), From a2cec1ad64ea314efe8f8d73e0e4117a7781f29e Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 28 Oct 2016 14:27:42 +0200 Subject: [PATCH 07/14] Create check_ref method to allow to check coercion with & types --- src/libcollections/lib.rs | 1 - src/libcollections/string.rs | 1 - src/librustc/infer/error_reporting.rs | 2 +- src/librustc_typeck/check/demand.rs | 116 ++++++++++---------- src/librustc_typeck/check/method/probe.rs | 16 ++- src/libsyntax/feature_gate.rs | 6 - src/test/compile-fail/coerce_suggestions.rs | 41 +++++++ 7 files changed, 114 insertions(+), 69 deletions(-) create mode 100644 src/test/compile-fail/coerce_suggestions.rs diff --git a/src/libcollections/lib.rs b/src/libcollections/lib.rs index 2abba4b344180..23d6edd6d794e 100644 --- a/src/libcollections/lib.rs +++ b/src/libcollections/lib.rs @@ -28,7 +28,6 @@ #![cfg_attr(test, allow(deprecated))] // rand #![cfg_attr(not(stage0), deny(warnings))] -#![cfg_attr(not(stage0), feature(safe_suggestion))] #![feature(alloc)] #![feature(allow_internal_unstable)] diff --git a/src/libcollections/string.rs b/src/libcollections/string.rs index 3712bd4b7ab8d..348eb6fb5ffa4 100644 --- a/src/libcollections/string.rs +++ b/src/libcollections/string.rs @@ -1234,7 +1234,6 @@ impl String { /// assert_eq!(a.len(), 3); /// ``` #[inline] - #[cfg_attr(not(stage0), safe_suggestion)] #[stable(feature = "rust1", since = "1.0.0")] pub fn len(&self) -> usize { self.vec.len() diff --git a/src/librustc/infer/error_reporting.rs b/src/librustc/infer/error_reporting.rs index c529c401914ca..47c0bc5fd60c5 100644 --- a/src/librustc/infer/error_reporting.rs +++ b/src/librustc/infer/error_reporting.rs @@ -548,7 +548,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { { let expected_found = match values { None => None, - Some(ref values) => match self.values_str(&values) { + Some(values) => match self.values_str(&values) { Some((expected, found)) => Some((expected, found)), None => { // Derived error. Cancel the emitter. diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 645bd31e1808f..91a6d45610ed3 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -18,28 +18,10 @@ use syntax_pos::{self, Span}; use rustc::hir; use rustc::ty::{self, ImplOrTraitItem}; -use hir::def_id::DefId; - use std::rc::Rc; use super::method::probe; -struct MethodInfo<'tcx> { - ast: Option, - id: DefId, - item: Rc>, -} - -impl<'tcx> MethodInfo<'tcx> { - fn new(ast: Option, id: DefId, item: Rc>) -> MethodInfo { - MethodInfo { - ast: ast, - id: id, - item: item, - } - } -} - impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // Requires that the two types unify, and prints an error message if // they don't. @@ -76,6 +58,46 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } } + fn check_ref(&self, expr: &hir::Expr, checked_ty: Ty<'tcx>, + expected: Ty<'tcx>) -> Option { + match (&checked_ty.sty, &expected.sty) { + (&ty::TyRef(_, x_mutability), &ty::TyRef(_, y_mutability)) => { + // check if there is a mutability difference + if x_mutability.mutbl == hir::Mutability::MutImmutable && + x_mutability.mutbl != y_mutability.mutbl && + self.can_sub_types(&x_mutability.ty, y_mutability.ty).is_ok() { + if let Ok(src) = self.tcx.sess.codemap().span_to_snippet(expr.span) { + return Some(format!("try with `&mut {}`", &src.replace("&", ""))); + } + } + None + } + (_, &ty::TyRef(_, mutability)) => { + // check if it can work when put into a ref + let ref_ty = match mutability.mutbl { + hir::Mutability::MutMutable => self.tcx.mk_mut_ref( + self.tcx.mk_region(ty::ReStatic), + checked_ty), + hir::Mutability::MutImmutable => self.tcx.mk_imm_ref( + self.tcx.mk_region(ty::ReStatic), + checked_ty), + }; + if self.try_coerce(expr, ref_ty, expected).is_ok() { + if let Ok(src) = self.tcx.sess.codemap().span_to_snippet(expr.span) { + return Some(format!("try with `{}{}`", + match mutability.mutbl { + hir::Mutability::MutMutable => "&mut ", + hir::Mutability::MutImmutable => "&", + }, + &src)); + } + } + None + } + _ => None, + } + } + // Checks that the type of `expr` can be coerced to `expected`. pub fn demand_coerce(&self, expr: &hir::Expr, checked_ty: Ty<'tcx>, expected: Ty<'tcx>) { let expected = self.resolve_type_vars_with_obligations(expected); @@ -83,34 +105,23 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let origin = TypeOrigin::Misc(expr.span); let expr_ty = self.resolve_type_vars_with_obligations(checked_ty); let mode = probe::Mode::MethodCall; - let suggestions = - if let Ok(methods) = self.probe_return(syntax_pos::DUMMY_SP, mode, expected, - checked_ty, ast::DUMMY_NODE_ID) { + let suggestions = if let Some(s) = self.check_ref(expr, checked_ty, expected) { + Some(s) + } else if let Ok(methods) = self.probe_return(syntax_pos::DUMMY_SP, + mode, + expected, + checked_ty, + ast::DUMMY_NODE_ID) { let suggestions: Vec<_> = methods.iter() - .filter_map(|ref x| { - if let Some(id) = self.get_impl_id(&x.item) { - Some(MethodInfo::new(None, id, Rc::new(x.item.clone()))) - } else { - None - }}) + .map(|ref x| { + Rc::new(x.item.clone()) + }) .collect(); if suggestions.len() > 0 { - let safe_suggestions: Vec<_> = - suggestions.iter() - .map(|ref x| MethodInfo::new( - self.find_attr(x.id, "safe_suggestion"), - x.id, - x.item.clone())) - .filter(|ref x| x.ast.is_some()) - .collect(); - Some(if safe_suggestions.len() > 0 { - self.get_best_match(&safe_suggestions) - } else { - format!("no safe suggestion found, here are functions which match your \ - needs but be careful:\n - {}", - self.get_best_match(&suggestions)) - }) + Some(format!("here are some functions which \ + might fulfill your needs:\n - {}", + self.get_best_match(&suggestions))) } else { None } @@ -125,34 +136,29 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } } - fn get_best_match(&self, methods: &[MethodInfo<'tcx>]) -> String { + fn get_best_match(&self, methods: &[Rc>]) -> String { if methods.len() == 1 { - return format!(" - {}", methods[0].item.name()); + return format!(" - {}", methods[0].name()); } - let no_argument_methods: Vec<&MethodInfo> = + let no_argument_methods: Vec<&Rc>> = methods.iter() - .filter(|ref x| self.has_not_input_arg(&*x.item)) + .filter(|ref x| self.has_not_input_arg(&*x)) .collect(); if no_argument_methods.len() > 0 { no_argument_methods.iter() - .map(|method| format!("{}", method.item.name())) + .take(5) + .map(|method| format!("{}", method.name())) .collect::>() .join("\n - ") } else { methods.iter() - .map(|method| format!("{}", method.item.name())) + .take(5) + .map(|method| format!("{}", method.name())) .collect::>() .join("\n - ") } } - fn get_impl_id(&self, impl_: &ImplOrTraitItem<'tcx>) -> Option { - match *impl_ { - ty::ImplOrTraitItem::MethodTraitItem(ref m) => Some((*m).def_id), - _ => None, - } - } - fn has_not_input_arg(&self, method: &ImplOrTraitItem<'tcx>) -> bool { match *method { ImplOrTraitItem::MethodTraitItem(ref x) => { diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index e6f9a176ed4e8..0290b98a873e3 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -192,7 +192,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // think cause spurious errors. Really though this part should // take place in the `self.probe` below. let steps = if mode == Mode::MethodCall { - match self.create_steps(span, self_ty) { + match self.create_steps(span, self_ty, &looking_for) { Some(steps) => steps, None => { return Err(MethodError::NoMatch(NoMatchData::new(Vec::new(), @@ -245,7 +245,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { fn create_steps(&self, span: Span, - self_ty: Ty<'tcx>) + self_ty: Ty<'tcx>, + looking_for: &LookingFor<'tcx>) -> Option>> { // FIXME: we don't need to create the entire steps in one pass @@ -260,7 +261,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { }) .collect(); - let final_ty = autoderef.unambiguous_final_ty(); + let final_ty = match looking_for { + &LookingFor::MethodName(_) => autoderef.unambiguous_final_ty(), + &LookingFor::ReturnType(_) => self_ty, + }; match final_ty.sty { ty::TyArray(elem_ty, _) => { let dereferences = steps.len() - 1; @@ -637,13 +641,15 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { } pub fn matches_return_type(&self, method: &ty::ImplOrTraitItem<'tcx>, - expected: ty::Ty<'tcx>) -> bool { + expected: ty::Ty<'tcx>) -> bool { match *method { ty::ImplOrTraitItem::MethodTraitItem(ref x) => { self.probe(|_| { let output = self.replace_late_bound_regions_with_fresh_var( self.span, infer::FnCall, &x.fty.sig.output()); - self.can_sub_types(output.0, expected).is_ok() + let substs = self.fresh_substs_for_item(self.span, method.def_id()); + let output = output.0.subst(self.tcx, substs); + self.can_sub_types(output, expected).is_ok() }) } _ => false, diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index cae1db80a99f6..db238353f6b88 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -652,12 +652,6 @@ pub const KNOWN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeGat "internal implementation detail", cfg_fn!(rustc_attrs))), - ("safe_suggestion", Whitelisted, Gated(Stability::Unstable, - "safe_suggestion", - "the `#[safe_suggestion]` attribute \ - is an experimental feature", - cfg_fn!(safe_suggestion))), - // FIXME: #14408 whitelist docs since rustdoc looks at them ("doc", Whitelisted, Ungated), diff --git a/src/test/compile-fail/coerce_suggestions.rs b/src/test/compile-fail/coerce_suggestions.rs new file mode 100644 index 0000000000000..decd589e6f4d8 --- /dev/null +++ b/src/test/compile-fail/coerce_suggestions.rs @@ -0,0 +1,41 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn test(_x: &mut String) {} +fn test2(_x: &mut i32) {} + +fn main() { + let x: usize = String::new(); + //^ ERROR E0308 + //| NOTE expected type `usize` + //| NOTE found type `std::string::String` + //| NOTE here are some functions which might fulfill your needs: + let x: &str = String::new(); + //^ ERROR E0308 + //| NOTE expected type `&str` + //| NOTE found type `std::string::String` + //| NOTE try with `&String::new()` + let y = String::new(); + test(&y); + //^ ERROR E0308 + //| NOTE expected type `&mut std::string::String` + //| NOTE found type `&std::string::String` + //| NOTE try with `&mut y` + test2(&y); + //^ ERROR E0308 + //| NOTE expected type `&mut i32` + //| NOTE found type `&std::string::String` + //| NOTE try with `&mut y` + let f; + f = box f; + //^ ERROR E0308 + //| NOTE expected type `_` + //| NOTE found type `Box<_>` +} From 19fb17cd9079be67d4e82741ee003cb2266819ec Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 30 Oct 2016 18:25:45 +0100 Subject: [PATCH 08/14] Fix invalid probe path --- src/librustc_typeck/check/method/probe.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 0290b98a873e3..2686731c76d9d 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -970,6 +970,12 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { for step in steps.iter() { match self.pick_step(step) { Some(Ok(mut elems)) => ret.append(&mut elems), + Some(Err(elem)) => { + match self.looking_for { + LookingFor::MethodName(_) => return Some(Err(elem)), + LookingFor::ReturnType(_) => {} + } + } _ => {} } } From 9c63817a5e023c3b36d9eb3fa000610a467a82c5 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 30 Oct 2016 23:06:30 +0100 Subject: [PATCH 09/14] fix tests --- src/librustc_typeck/check/method/probe.rs | 119 +++++++++++--------- src/test/compile-fail/coerce_suggestions.rs | 44 ++++---- src/test/compile-fail/coercion-slice.rs | 2 +- src/test/compile-fail/cross-borrow-trait.rs | 1 + src/test/compile-fail/dst-bad-coercions.rs | 2 + src/test/compile-fail/issue-13058.rs | 1 + 6 files changed, 94 insertions(+), 75 deletions(-) diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 2686731c76d9d..92012c78f3112 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -283,15 +283,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { Some(steps) } - - pub fn find_attr(&self, def_id: DefId, attr_name: &str) -> Option { - for item in self.tcx.get_attrs(def_id).iter() { - if item.check_name(attr_name) { - return Some(item.clone()); - } - } - None - } } impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { @@ -926,21 +917,21 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { Some(Ok(p)) => p.iter().map(|p| p.item.container().id()).collect(), Some(Err(MethodError::Ambiguity(v))) => { v.into_iter() - .map(|source| { - match source { - TraitSource(id) => id, - ImplSource(impl_id) => { - match tcx.trait_id_of_impl(impl_id) { - Some(id) => id, - None => { - span_bug!(span, - "found inherent method when looking at traits") - } + .map(|source| { + match source { + TraitSource(id) => id, + ImplSource(impl_id) => { + match tcx.trait_id_of_impl(impl_id) { + Some(id) => id, + None => { + span_bug!(span, + "found inherent method when looking at traits") } } } - }) - .collect() + } + }) + .collect() } Some(Err(MethodError::NoMatch(NoMatchData { out_of_scope_traits: others, .. }))) => { assert!(others.is_empty()); @@ -965,25 +956,27 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { fn pick_core(&mut self) -> Option> { let steps = self.steps.clone(); - let mut ret = Vec::new(); - for step in steps.iter() { - match self.pick_step(step) { - Some(Ok(mut elems)) => ret.append(&mut elems), - Some(Err(elem)) => { - match self.looking_for { - LookingFor::MethodName(_) => return Some(Err(elem)), - LookingFor::ReturnType(_) => {} + match self.looking_for { + LookingFor::MethodName(_) => steps.iter() + .filter_map(|step| self.pick_step(step)) + .next(), + LookingFor::ReturnType(_) => { + let mut ret = Vec::new(); + + for step in steps.iter() { + match self.pick_step(step) { + Some(Ok(mut elems)) => ret.append(&mut elems), + _ => {} } } - _ => {} + if ret.len() < 1 { + None + } else { + Some(Ok(ret)) + } } } - if ret.len() < 1 { - None - } else { - Some(Ok(ret)) - } } fn pick_step(&mut self, step: &CandidateStep<'tcx>) -> Option> { @@ -1036,12 +1029,12 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { // In general, during probing we erase regions. See // `impl_self_ty()` for an explanation. let region = tcx.mk_region(ty::ReErased); - let mut res = Vec::new(); // Search through mutabilities in order to find one where pick works: - for _ in [hir::MutImmutable, hir::MutMutable] - .iter() - .filter_map(|&m| { + let mut elements = [hir::MutImmutable, hir::MutMutable]; + let mut it = elements + .iter_mut() + .filter_map(|&mut m| { let autoref_ty = tcx.mk_ref(region, ty::TypeAndMut { ty: step.self_ty, @@ -1058,15 +1051,24 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { None }; } - res.append(&mut picks); + picks }) }) - }) {} - - if res.len() < 1 { - None - } else { - Some(Ok(res)) + }); + match self.looking_for { + LookingFor::MethodName(_) => it.nth(0), + LookingFor::ReturnType(_) => { + let mut ret = Vec::new(); + it.filter_map(|entry| entry.ok()) + .map(|mut v| { ret.append(&mut v); }) + .all(|_| true); + + if ret.len() < 1 { + None + } else { + Some(Ok(ret)) + } + } } } @@ -1097,7 +1099,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { probes: &[Candidate<'tcx>], possibly_unsatisfied_predicates: &mut Vec>) -> Option> { - let applicable_candidates: Vec<_> = probes.iter() + let mut applicable_candidates: Vec<_> = probes.iter() .filter(|&probe| self.consider_probe(self_ty, probe, possibly_unsatisfied_predicates)) .collect(); @@ -1117,14 +1119,21 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { return Some(Err(MethodError::Ambiguity(sources))); } - let ret: Vec<_> = applicable_candidates.iter() - .map(|probe| probe.to_unadjusted_pick()) - .collect(); - - if ret.len() < 1 { - None - } else { - Some(Ok(ret)) + match self.looking_for { + LookingFor::MethodName(_) => applicable_candidates + .pop() + .map(|probe| Ok(vec![probe.to_unadjusted_pick()])), + LookingFor::ReturnType(_) => { + let ret: Vec<_> = applicable_candidates.iter() + .map(|probe| probe.to_unadjusted_pick()) + .collect(); + + if ret.len() < 1 { + None + } else { + Some(Ok(ret)) + } + } } } diff --git a/src/test/compile-fail/coerce_suggestions.rs b/src/test/compile-fail/coerce_suggestions.rs index decd589e6f4d8..3177e858ff4fd 100644 --- a/src/test/compile-fail/coerce_suggestions.rs +++ b/src/test/compile-fail/coerce_suggestions.rs @@ -8,34 +8,40 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#![feature(box_syntax)] + fn test(_x: &mut String) {} fn test2(_x: &mut i32) {} fn main() { let x: usize = String::new(); - //^ ERROR E0308 - //| NOTE expected type `usize` - //| NOTE found type `std::string::String` - //| NOTE here are some functions which might fulfill your needs: + //~^ ERROR E0308 + //~| NOTE expected usize, found struct `std::string::String` + //~| NOTE expected type `usize` + //~| NOTE found type `std::string::String` + //~| HELP here are some functions which might fulfill your needs: let x: &str = String::new(); - //^ ERROR E0308 - //| NOTE expected type `&str` - //| NOTE found type `std::string::String` - //| NOTE try with `&String::new()` + //~^ ERROR E0308 + //~| NOTE expected &str, found struct `std::string::String` + //~| NOTE expected type `&str` + //~| NOTE found type `std::string::String` + //~| HELP try with `&String::new()` let y = String::new(); test(&y); - //^ ERROR E0308 - //| NOTE expected type `&mut std::string::String` - //| NOTE found type `&std::string::String` - //| NOTE try with `&mut y` + //~^ ERROR E0308 + //~| NOTE types differ in mutability + //~| NOTE expected type `&mut std::string::String` + //~| NOTE found type `&std::string::String` + //~| HELP try with `&mut y` test2(&y); - //^ ERROR E0308 - //| NOTE expected type `&mut i32` - //| NOTE found type `&std::string::String` - //| NOTE try with `&mut y` + //~^ ERROR E0308 + //~| NOTE types differ in mutability + //~| NOTE expected type `&mut i32` + //~| NOTE found type `&std::string::String` let f; f = box f; - //^ ERROR E0308 - //| NOTE expected type `_` - //| NOTE found type `Box<_>` + //~^ ERROR E0308 + //~| NOTE cyclic type of infinite size + //~| NOTE expected type `_` + //~| NOTE found type `Box<_>` } diff --git a/src/test/compile-fail/coercion-slice.rs b/src/test/compile-fail/coercion-slice.rs index 6b468ff96620d..f50900883999c 100644 --- a/src/test/compile-fail/coercion-slice.rs +++ b/src/test/compile-fail/coercion-slice.rs @@ -14,6 +14,6 @@ fn main() { let _: &[i32] = [0]; //~^ ERROR mismatched types //~| expected type `&[i32]` - //~| found type `[{integer}; 1]` + //~| found type `[i32; 1]` //~| expected &[i32], found array of 1 elements } diff --git a/src/test/compile-fail/cross-borrow-trait.rs b/src/test/compile-fail/cross-borrow-trait.rs index 672ff464718f8..975bc1300aae7 100644 --- a/src/test/compile-fail/cross-borrow-trait.rs +++ b/src/test/compile-fail/cross-borrow-trait.rs @@ -22,4 +22,5 @@ pub fn main() { //~| expected type `&Trait` //~| found type `Box` //~| expected &Trait, found box + //~^^^^ ERROR E0277 } diff --git a/src/test/compile-fail/dst-bad-coercions.rs b/src/test/compile-fail/dst-bad-coercions.rs index 883c16b089581..728b016b30fa5 100644 --- a/src/test/compile-fail/dst-bad-coercions.rs +++ b/src/test/compile-fail/dst-bad-coercions.rs @@ -23,11 +23,13 @@ pub fn main() { let x: *const S = &S; let y: &S = x; //~ ERROR mismatched types let y: &T = x; //~ ERROR mismatched types + //~^ ERROR E0277 // Test that we cannot convert from *-ptr to &S and &T (mut version) let x: *mut S = &mut S; let y: &S = x; //~ ERROR mismatched types let y: &T = x; //~ ERROR mismatched types + //~^ ERROR E0277 // Test that we cannot convert an immutable ptr to a mutable one using *-ptrs let x: &mut T = &S; //~ ERROR mismatched types diff --git a/src/test/compile-fail/issue-13058.rs b/src/test/compile-fail/issue-13058.rs index de578257e4684..df6675510ff2b 100644 --- a/src/test/compile-fail/issue-13058.rs +++ b/src/test/compile-fail/issue-13058.rs @@ -36,4 +36,5 @@ fn check<'r, I: Iterator, T: Itble<'r, usize, I>>(cont: &T) -> bool fn main() { check((3, 5)); //~^ ERROR mismatched types +//~| HELP try with `&(3, 5)` } From 82d31c46141cfb5e8d6686c6bb843fdb8e1c2ebe Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 1 Nov 2016 16:00:24 +0100 Subject: [PATCH 10/14] improve suggestions output and add ui test --- src/librustc_typeck/check/demand.rs | 4 +- .../span/coerce-suggestions.rs} | 0 src/test/ui/span/coerce-suggestions.stderr | 52 +++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) rename src/test/{compile-fail/coerce_suggestions.rs => ui/span/coerce-suggestions.rs} (100%) create mode 100644 src/test/ui/span/coerce-suggestions.stderr diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 91a6d45610ed3..e832c6a70dc24 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -147,13 +147,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if no_argument_methods.len() > 0 { no_argument_methods.iter() .take(5) - .map(|method| format!("{}", method.name())) + .map(|method| format!(".{}()", method.name())) .collect::>() .join("\n - ") } else { methods.iter() .take(5) - .map(|method| format!("{}", method.name())) + .map(|method| format!(".{}()", method.name())) .collect::>() .join("\n - ") } diff --git a/src/test/compile-fail/coerce_suggestions.rs b/src/test/ui/span/coerce-suggestions.rs similarity index 100% rename from src/test/compile-fail/coerce_suggestions.rs rename to src/test/ui/span/coerce-suggestions.rs diff --git a/src/test/ui/span/coerce-suggestions.stderr b/src/test/ui/span/coerce-suggestions.stderr new file mode 100644 index 0000000000000..3673228301403 --- /dev/null +++ b/src/test/ui/span/coerce-suggestions.stderr @@ -0,0 +1,52 @@ +error[E0308]: mismatched types + --> $DIR/coerce-suggestions.rs:17:20 + | +17 | let x: usize = String::new(); + | ^^^^^^^^^^^^^ expected usize, found struct `std::string::String` + | + = note: expected type `usize` + = note: found type `std::string::String` + = help: here are some functions which might fulfill your needs: + - .capacity() + - .len() + +error[E0308]: mismatched types + --> $DIR/coerce-suggestions.rs:23:19 + | +23 | let x: &str = String::new(); + | ^^^^^^^^^^^^^ expected &str, found struct `std::string::String` + | + = note: expected type `&str` + = note: found type `std::string::String` + = help: try with `&String::new()` + +error[E0308]: mismatched types + --> $DIR/coerce-suggestions.rs:30:10 + | +30 | test(&y); + | ^^ types differ in mutability + | + = note: expected type `&mut std::string::String` + = note: found type `&std::string::String` + = help: try with `&mut y` + +error[E0308]: mismatched types + --> $DIR/coerce-suggestions.rs:36:11 + | +36 | test2(&y); + | ^^ types differ in mutability + | + = note: expected type `&mut i32` + = note: found type `&std::string::String` + +error[E0308]: mismatched types + --> $DIR/coerce-suggestions.rs:42:9 + | +42 | f = box f; + | ^^^^^ cyclic type of infinite size + | + = note: expected type `_` + = note: found type `Box<_>` + +error: aborting due to 5 previous errors + From a9defa839eb5415c5d7bd830078a0ec98050cdf3 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 2 Nov 2016 00:03:45 +0100 Subject: [PATCH 11/14] splitting methods into smaller ones, add docs, better variable naming --- src/librustc_typeck/check/demand.rs | 93 ++++++++++++++++------- src/librustc_typeck/check/method/mod.rs | 6 +- src/librustc_typeck/check/method/probe.rs | 74 ++++++++++-------- 3 files changed, 109 insertions(+), 64 deletions(-) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index e832c6a70dc24..f532e5b916827 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -58,22 +58,50 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } } - fn check_ref(&self, expr: &hir::Expr, checked_ty: Ty<'tcx>, - expected: Ty<'tcx>) -> Option { - match (&checked_ty.sty, &expected.sty) { - (&ty::TyRef(_, x_mutability), &ty::TyRef(_, y_mutability)) => { + /// This function is used to determine potential "simple" improvements or users' errors and + /// provide them useful help. For example: + /// + /// ``` + /// fn some_fn(s: &str) {} + /// + /// let x = "hey!".to_owned(); + /// some_fn(x); // error + /// ``` + /// + /// No need to find every potential function which could make a coercion to transform a + /// `String` into a `&str` since a `&` would do the trick! + /// + /// In addition of this check, it also checks between references mutability state. If the + /// expected is mutable but the provided isn't, maybe we could just say "Hey, try with + /// `&mut`!". + fn check_ref(&self, + expr: &hir::Expr, + checked_ty: Ty<'tcx>, + expected: Ty<'tcx>) + -> Option { + match (&expected.sty, &checked_ty.sty) { + (&ty::TyRef(_, expected_mutability), + &ty::TyRef(_, checked_mutability)) => { // check if there is a mutability difference - if x_mutability.mutbl == hir::Mutability::MutImmutable && - x_mutability.mutbl != y_mutability.mutbl && - self.can_sub_types(&x_mutability.ty, y_mutability.ty).is_ok() { + if checked_mutability.mutbl == hir::Mutability::MutImmutable && + checked_mutability.mutbl != expected_mutability.mutbl && + self.can_sub_types(&checked_mutability.ty, + expected_mutability.ty).is_ok() { if let Ok(src) = self.tcx.sess.codemap().span_to_snippet(expr.span) { return Some(format!("try with `&mut {}`", &src.replace("&", ""))); } } None } - (_, &ty::TyRef(_, mutability)) => { - // check if it can work when put into a ref + (&ty::TyRef(_, mutability), _) => { + // Check if it can work when put into a ref. For example: + // + // ``` + // fn bar(x: &mut i32) {} + // + // let x = 0u32; + // bar(&x); // error, expected &mut + // ``` let ref_ty = match mutability.mutbl { hir::Mutability::MutMutable => self.tcx.mk_mut_ref( self.tcx.mk_region(ty::ReStatic), @@ -107,11 +135,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let mode = probe::Mode::MethodCall; let suggestions = if let Some(s) = self.check_ref(expr, checked_ty, expected) { Some(s) - } else if let Ok(methods) = self.probe_return(syntax_pos::DUMMY_SP, - mode, - expected, - checked_ty, - ast::DUMMY_NODE_ID) { + } else if let Ok(methods) = self.probe_for_return_type(syntax_pos::DUMMY_SP, + mode, + expected, + checked_ty, + ast::DUMMY_NODE_ID) { let suggestions: Vec<_> = methods.iter() .map(|ref x| { @@ -136,29 +164,38 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } } + fn format_method_suggestion(&self, method: &ImplOrTraitItem<'tcx>) -> String { + format!(".{}({})", + method.name(), + if self.has_not_input_arg(method) { + "" + } else { + "..." + }) + } + + fn display_suggested_methods(&self, methods: &[Rc>]) -> String { + methods.iter() + .take(5) + .map(|method| self.format_method_suggestion(&*method)) + .collect::>() + .join("\n - ") + } + fn get_best_match(&self, methods: &[Rc>]) -> String { - if methods.len() == 1 { - return format!(" - {}", methods[0].name()); - } - let no_argument_methods: Vec<&Rc>> = + let no_argument_methods: Vec>> = methods.iter() .filter(|ref x| self.has_not_input_arg(&*x)) + .map(|x| x.clone()) .collect(); if no_argument_methods.len() > 0 { - no_argument_methods.iter() - .take(5) - .map(|method| format!(".{}()", method.name())) - .collect::>() - .join("\n - ") + self.display_suggested_methods(&no_argument_methods) } else { - methods.iter() - .take(5) - .map(|method| format!(".{}()", method.name())) - .collect::>() - .join("\n - ") + self.display_suggested_methods(&methods) } } + // This function checks if the method isn't static and takes other arguments than `self`. fn has_not_input_arg(&self, method: &ImplOrTraitItem<'tcx>) -> bool { match *method { ImplOrTraitItem::MethodTraitItem(ref x) => { diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index cddd097b3f6d5..0dfedba0ea9d9 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -91,7 +91,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { allow_private: bool) -> bool { let mode = probe::Mode::MethodCall; - match self.probe_method(span, mode, method_name, self_ty, call_expr_id) { + match self.probe_for_name(span, mode, method_name, self_ty, call_expr_id) { Ok(..) => true, Err(NoMatch(..)) => false, Err(Ambiguity(..)) => true, @@ -130,7 +130,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let mode = probe::Mode::MethodCall; let self_ty = self.resolve_type_vars_if_possible(&self_ty); - let pick = self.probe_method(span, mode, method_name, self_ty, call_expr.id)?.remove(0); + let pick = self.probe_for_name(span, mode, method_name, self_ty, call_expr.id)?.remove(0); if let Some(import_id) = pick.import_id { self.tcx.used_trait_imports.borrow_mut().insert(import_id); @@ -353,7 +353,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { expr_id: ast::NodeId) -> Result> { let mode = probe::Mode::Path; - let picks = self.probe_method(span, mode, method_name, self_ty, expr_id)?; + let picks = self.probe_for_name(span, mode, method_name, self_ty, expr_id)?; let pick = &picks[0]; if let Some(import_id) = pick.import_id { diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 92012c78f3112..7b6212dce2f3b 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -148,41 +148,41 @@ pub enum Mode { } impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { - pub fn probe_return(&self, - span: Span, - mode: Mode, - return_type: Ty<'tcx>, - self_ty: Ty<'tcx>, - scope_expr_id: ast::NodeId) - -> PickResult<'tcx> { + pub fn probe_for_return_type(&self, + span: Span, + mode: Mode, + return_type: Ty<'tcx>, + self_ty: Ty<'tcx>, + scope_expr_id: ast::NodeId) + -> PickResult<'tcx> { debug!("probe(self_ty={:?}, return_type={}, scope_expr_id={})", self_ty, return_type, scope_expr_id); - self._probe(span, mode, LookingFor::ReturnType(return_type), self_ty, scope_expr_id) + self.probe_for(span, mode, LookingFor::ReturnType(return_type), self_ty, scope_expr_id) } - pub fn probe_method(&self, - span: Span, - mode: Mode, - item_name: ast::Name, - self_ty: Ty<'tcx>, - scope_expr_id: ast::NodeId) - -> PickResult<'tcx> { + pub fn probe_for_name(&self, + span: Span, + mode: Mode, + item_name: ast::Name, + self_ty: Ty<'tcx>, + scope_expr_id: ast::NodeId) + -> PickResult<'tcx> { debug!("probe(self_ty={:?}, item_name={}, scope_expr_id={})", self_ty, item_name, scope_expr_id); - self._probe(span, mode, LookingFor::MethodName(item_name), self_ty, scope_expr_id) + self.probe_for(span, mode, LookingFor::MethodName(item_name), self_ty, scope_expr_id) } - fn _probe(&self, - span: Span, - mode: Mode, - looking_for: LookingFor<'tcx>, - self_ty: Ty<'tcx>, - scope_expr_id: ast::NodeId) - -> PickResult<'tcx> { + fn probe_for(&self, + span: Span, + mode: Mode, + looking_for: LookingFor<'tcx>, + self_ty: Ty<'tcx>, + scope_expr_id: ast::NodeId) + -> PickResult<'tcx> { // FIXME(#18741) -- right now, creating the steps involves evaluating the // `*` operator, which registers obligations that then escape into @@ -263,6 +263,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let final_ty = match looking_for { &LookingFor::MethodName(_) => autoderef.unambiguous_final_ty(), + // Since ReturnType case tries to coerce the returned type to the + // expected one, we need all the information! &LookingFor::ReturnType(_) => self_ty, }; match final_ty.sty { @@ -636,10 +638,10 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { match *method { ty::ImplOrTraitItem::MethodTraitItem(ref x) => { self.probe(|_| { - let output = self.replace_late_bound_regions_with_fresh_var( - self.span, infer::FnCall, &x.fty.sig.output()); let substs = self.fresh_substs_for_item(self.span, method.def_id()); - let output = output.0.subst(self.tcx, substs); + let output = x.fty.sig.output().subst(self.tcx, substs); + let (output, _) = self.replace_late_bound_regions_with_fresh_var( + self.span, infer::FnCall, &output); self.can_sub_types(output, expected).is_ok() }) } @@ -958,10 +960,17 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { let steps = self.steps.clone(); match self.looking_for { - LookingFor::MethodName(_) => steps.iter() - .filter_map(|step| self.pick_step(step)) - .next(), + LookingFor::MethodName(_) => { + // find the first step that works + steps.iter() + .filter_map(|step| self.pick_step(step)) + .next() + } LookingFor::ReturnType(_) => { + // Normally, we stop at the first step where we find a positive match. + // But when we are scanning for methods with a suitable return type, + // these methods have distinct names and hence may not shadow one another + // (also, this is just for hints, so precision is less important). let mut ret = Vec::new(); for step in steps.iter() { @@ -1058,10 +1067,9 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { match self.looking_for { LookingFor::MethodName(_) => it.nth(0), LookingFor::ReturnType(_) => { - let mut ret = Vec::new(); - it.filter_map(|entry| entry.ok()) - .map(|mut v| { ret.append(&mut v); }) - .all(|_| true); + let ret = it.filter_map(|entry| entry.ok()) + .flat_map(|v| v) + .collect::>(); if ret.len() < 1 { None From 7d513b9a64c33ee17f8aef20a75da5a5abfdbdff Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 22 Nov 2016 10:47:18 -0500 Subject: [PATCH 12/14] make `probe_for` generic over final operation --- src/librustc_typeck/check/method/probe.rs | 30 ++++++++++++++--------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 7b6212dce2f3b..8b1a6d771ef41 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -159,7 +159,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self_ty, return_type, scope_expr_id); - self.probe_for(span, mode, LookingFor::ReturnType(return_type), self_ty, scope_expr_id) + self.probe_op(span, mode, LookingFor::ReturnType(return_type), self_ty, scope_expr_id, + |probe_cx| probe_cx.pick()) } pub fn probe_for_name(&self, @@ -173,17 +174,24 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self_ty, item_name, scope_expr_id); - self.probe_for(span, mode, LookingFor::MethodName(item_name), self_ty, scope_expr_id) + self.probe_op(span, + mode, + LookingFor::MethodName(item_name), + self_ty, + scope_expr_id, + |probe_cx| probe_cx.pick()) } - fn probe_for(&self, - span: Span, - mode: Mode, - looking_for: LookingFor<'tcx>, - self_ty: Ty<'tcx>, - scope_expr_id: ast::NodeId) - -> PickResult<'tcx> { - + fn probe_op<'a,OP,R>(&'a self, + span: Span, + mode: Mode, + looking_for: LookingFor<'tcx>, + self_ty: Ty<'tcx>, + scope_expr_id: ast::NodeId, + op: OP) + -> R + where OP: FnOnce(&mut ProbeContext<'a, 'gcx, 'tcx>) -> R + { // FIXME(#18741) -- right now, creating the steps involves evaluating the // `*` operator, which registers obligations that then escape into // the global fulfillment context and thus has global @@ -239,7 +247,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { steps, opt_simplified_steps); probe_cx.assemble_inherent_candidates(); probe_cx.assemble_extension_candidates_for_traits_in_scope(scope_expr_id)?; - probe_cx.pick() + op(&mut probe_cx) }) } From 6ade7acf145b372950f328a1cd192a86c4333042 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 22 Nov 2016 15:36:28 -0500 Subject: [PATCH 13/14] rewrite return type probing to use the "probe by name" path We now do two phases. First, we gather up the list of candidates with suitable return types and extract their names. Then we filter those to see which are applicable and we return that. It might be nice to do the "filter by return type" as a second step, but this is ok for now. --- src/librustc_typeck/check/demand.rs | 27 +++---- src/librustc_typeck/check/method/probe.rs | 94 +++++++++++++---------- 2 files changed, 63 insertions(+), 58 deletions(-) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index f532e5b916827..56fc970dc3cfb 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -18,8 +18,6 @@ use syntax_pos::{self, Span}; use rustc::hir; use rustc::ty::{self, ImplOrTraitItem}; -use std::rc::Rc; - use super::method::probe; impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { @@ -135,17 +133,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let mode = probe::Mode::MethodCall; let suggestions = if let Some(s) = self.check_ref(expr, checked_ty, expected) { Some(s) - } else if let Ok(methods) = self.probe_for_return_type(syntax_pos::DUMMY_SP, - mode, - expected, - checked_ty, - ast::DUMMY_NODE_ID) { - let suggestions: Vec<_> = - methods.iter() - .map(|ref x| { - Rc::new(x.item.clone()) - }) - .collect(); + } else { + let suggestions = self.probe_for_return_type(syntax_pos::DUMMY_SP, + mode, + expected, + checked_ty, + ast::DUMMY_NODE_ID); if suggestions.len() > 0 { Some(format!("here are some functions which \ might fulfill your needs:\n - {}", @@ -153,8 +146,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } else { None } - } else { - None }; let mut err = self.report_mismatched_types(origin, expected, expr_ty, e); if let Some(suggestions) = suggestions { @@ -174,7 +165,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { }) } - fn display_suggested_methods(&self, methods: &[Rc>]) -> String { + fn display_suggested_methods(&self, methods: &[ImplOrTraitItem<'tcx>]) -> String { methods.iter() .take(5) .map(|method| self.format_method_suggestion(&*method)) @@ -182,8 +173,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { .join("\n - ") } - fn get_best_match(&self, methods: &[Rc>]) -> String { - let no_argument_methods: Vec>> = + fn get_best_match(&self, methods: &[ImplOrTraitItem<'tcx>]) -> String { + let no_argument_methods: Vec<_> = methods.iter() .filter(|ref x| self.has_not_input_arg(&*x)) .map(|x| x.clone()) diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 8b1a6d771ef41..91ec92a44aea5 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -148,19 +148,36 @@ pub enum Mode { } impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { + /// This is used to offer suggestions to users. It returns methods + /// that could have been called which have the desired return + /// type. Some effort is made to rule out methods that, if called, + /// would result in an error (basically, the same criteria we + /// would use to decide if a method is a plausible fit for + /// ambiguity purposes). pub fn probe_for_return_type(&self, span: Span, mode: Mode, return_type: Ty<'tcx>, self_ty: Ty<'tcx>, scope_expr_id: ast::NodeId) - -> PickResult<'tcx> { + -> Vec> { debug!("probe(self_ty={:?}, return_type={}, scope_expr_id={})", self_ty, return_type, scope_expr_id); - self.probe_op(span, mode, LookingFor::ReturnType(return_type), self_ty, scope_expr_id, - |probe_cx| probe_cx.pick()) + let method_names = + self.probe_op(span, mode, LookingFor::ReturnType(return_type), self_ty, scope_expr_id, + |probe_cx| Ok(probe_cx.candidate_method_names())) + .unwrap_or(vec![]); + method_names + .iter() + .flat_map(|&method_name| { + match self.probe_for_name(span, mode, method_name, self_ty, scope_expr_id) { + Ok(picks) => picks.into_iter().map(move |pick| pick.item).collect(), + Err(_) => vec![], + } + }) + .collect() } pub fn probe_for_name(&self, @@ -182,15 +199,15 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { |probe_cx| probe_cx.pick()) } - fn probe_op<'a,OP,R>(&'a self, - span: Span, - mode: Mode, - looking_for: LookingFor<'tcx>, - self_ty: Ty<'tcx>, - scope_expr_id: ast::NodeId, - op: OP) - -> R - where OP: FnOnce(&mut ProbeContext<'a, 'gcx, 'tcx>) -> R + fn probe_op(&'a self, + span: Span, + mode: Mode, + looking_for: LookingFor<'tcx>, + self_ty: Ty<'tcx>, + scope_expr_id: ast::NodeId, + op: OP) + -> Result> + where OP: FnOnce(ProbeContext<'a, 'gcx, 'tcx>) -> Result> { // FIXME(#18741) -- right now, creating the steps involves evaluating the // `*` operator, which registers obligations that then escape into @@ -247,7 +264,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { steps, opt_simplified_steps); probe_cx.assemble_inherent_candidates(); probe_cx.assemble_extension_candidates_for_traits_in_scope(scope_expr_id)?; - op(&mut probe_cx) + op(probe_cx) }) } @@ -903,10 +920,30 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { } } + fn candidate_method_names(&self) -> Vec { + let mut set = FnvHashSet(); + let mut names: Vec<_> = + self.inherent_candidates + .iter() + .chain(&self.extension_candidates) + .map(|candidate| candidate.item.name()) + .filter(|&name| set.insert(name)) + .collect(); + + // sort them by the name so we have a stable result + names.sort_by_key(|n| n.as_str()); + names + } + /////////////////////////////////////////////////////////////////////////// // THE ACTUAL SEARCH fn pick(mut self) -> PickResult<'tcx> { + assert!(match self.looking_for { + LookingFor::MethodName(_) => true, + LookingFor::ReturnType(_) => false, + }); + if let Some(ret) = self.pick_core() { return ret; } @@ -967,33 +1004,10 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { fn pick_core(&mut self) -> Option> { let steps = self.steps.clone(); - match self.looking_for { - LookingFor::MethodName(_) => { - // find the first step that works - steps.iter() - .filter_map(|step| self.pick_step(step)) - .next() - } - LookingFor::ReturnType(_) => { - // Normally, we stop at the first step where we find a positive match. - // But when we are scanning for methods with a suitable return type, - // these methods have distinct names and hence may not shadow one another - // (also, this is just for hints, so precision is less important). - let mut ret = Vec::new(); - - for step in steps.iter() { - match self.pick_step(step) { - Some(Ok(mut elems)) => ret.append(&mut elems), - _ => {} - } - } - if ret.len() < 1 { - None - } else { - Some(Ok(ret)) - } - } - } + // find the first step that works + steps.iter() + .filter_map(|step| self.pick_step(step)) + .next() } fn pick_step(&mut self, step: &CandidateStep<'tcx>) -> Option> { From ef4d5e815ccdcd6d6e77a0df55d69a406615db9c Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 22 Nov 2016 16:05:13 -0500 Subject: [PATCH 14/14] revert a lot of the changes to method probe internals --- src/librustc_typeck/check/method/mod.rs | 5 +- src/librustc_typeck/check/method/probe.rs | 133 ++++++++-------------- 2 files changed, 48 insertions(+), 90 deletions(-) diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index 0dfedba0ea9d9..d204b835d4f71 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -130,7 +130,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let mode = probe::Mode::MethodCall; let self_ty = self.resolve_type_vars_if_possible(&self_ty); - let pick = self.probe_for_name(span, mode, method_name, self_ty, call_expr.id)?.remove(0); + let pick = self.probe_for_name(span, mode, method_name, self_ty, call_expr.id)?; if let Some(import_id) = pick.import_id { self.tcx.used_trait_imports.borrow_mut().insert(import_id); @@ -353,8 +353,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { expr_id: ast::NodeId) -> Result> { let mode = probe::Mode::Path; - let picks = self.probe_for_name(span, mode, method_name, self_ty, expr_id)?; - let pick = &picks[0]; + let pick = self.probe_for_name(span, mode, method_name, self_ty, expr_id)?; if let Some(import_id) = pick.import_id { self.tcx.used_trait_imports.borrow_mut().insert(import_id); diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 91ec92a44aea5..de6f66ef7dd5c 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -133,7 +133,7 @@ pub enum PickKind<'tcx> { ty::PolyTraitRef<'tcx>), } -pub type PickResult<'tcx> = Result>, MethodError<'tcx>>; +pub type PickResult<'tcx> = Result, MethodError<'tcx>>; #[derive(PartialEq, Eq, Copy, Clone, Debug)] pub enum Mode { @@ -173,8 +173,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { .iter() .flat_map(|&method_name| { match self.probe_for_name(span, mode, method_name, self_ty, scope_expr_id) { - Ok(picks) => picks.into_iter().map(move |pick| pick.item).collect(), - Err(_) => vec![], + Ok(pick) => Some(pick.item), + Err(_) => None, } }) .collect() @@ -217,7 +217,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // think cause spurious errors. Really though this part should // take place in the `self.probe` below. let steps = if mode == Mode::MethodCall { - match self.create_steps(span, self_ty, &looking_for) { + match self.create_steps(span, self_ty) { Some(steps) => steps, None => { return Err(MethodError::NoMatch(NoMatchData::new(Vec::new(), @@ -270,8 +270,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { fn create_steps(&self, span: Span, - self_ty: Ty<'tcx>, - looking_for: &LookingFor<'tcx>) + self_ty: Ty<'tcx>) -> Option>> { // FIXME: we don't need to create the entire steps in one pass @@ -286,12 +285,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { }) .collect(); - let final_ty = match looking_for { - &LookingFor::MethodName(_) => autoderef.unambiguous_final_ty(), - // Since ReturnType case tries to coerce the returned type to the - // expected one, we need all the information! - &LookingFor::ReturnType(_) => self_ty, - }; + let final_ty = autoderef.unambiguous_final_ty(); match final_ty.sty { ty::TyArray(elem_ty, _) => { let dereferences = steps.len() - 1; @@ -944,8 +938,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { LookingFor::ReturnType(_) => false, }); - if let Some(ret) = self.pick_core() { - return ret; + if let Some(r) = self.pick_core() { + return r; } let static_candidates = mem::replace(&mut self.static_candidates, vec![]); @@ -961,24 +955,24 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { self.assemble_extension_candidates_for_all_traits()?; let out_of_scope_traits = match self.pick_core() { - Some(Ok(p)) => p.iter().map(|p| p.item.container().id()).collect(), + Some(Ok(p)) => vec![p.item.container().id()], Some(Err(MethodError::Ambiguity(v))) => { v.into_iter() - .map(|source| { - match source { - TraitSource(id) => id, - ImplSource(impl_id) => { - match tcx.trait_id_of_impl(impl_id) { - Some(id) => id, - None => { - span_bug!(span, - "found inherent method when looking at traits") + .map(|source| { + match source { + TraitSource(id) => id, + ImplSource(impl_id) => { + match tcx.trait_id_of_impl(impl_id) { + Some(id) => id, + None => { + span_bug!(span, + "found inherent method when looking at traits") + } } } } - } - }) - .collect() + }) + .collect() } Some(Err(MethodError::NoMatch(NoMatchData { out_of_scope_traits: others, .. }))) => { assert!(others.is_empty()); @@ -1005,9 +999,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { let steps = self.steps.clone(); // find the first step that works - steps.iter() - .filter_map(|step| self.pick_step(step)) - .next() + steps.iter().filter_map(|step| self.pick_step(step)).next() } fn pick_step(&mut self, step: &CandidateStep<'tcx>) -> Option> { @@ -1038,18 +1030,16 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { } self.pick_method(step.self_ty).map(|r| { - r.map(|mut picks| { - for pick in picks.iter_mut() { - pick.autoderefs = step.autoderefs; - - // Insert a `&*` or `&mut *` if this is a reference type: - if let ty::TyRef(_, mt) = step.self_ty.sty { - pick.autoderefs += 1; - pick.autoref = Some(mt.mutbl); - } + r.map(|mut pick| { + pick.autoderefs = step.autoderefs; + + // Insert a `&*` or `&mut *` if this is a reference type: + if let ty::TyRef(_, mt) = step.self_ty.sty { + pick.autoderefs += 1; + pick.autoref = Some(mt.mutbl); } - picks + pick }) }) } @@ -1062,44 +1052,28 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { let region = tcx.mk_region(ty::ReErased); // Search through mutabilities in order to find one where pick works: - let mut elements = [hir::MutImmutable, hir::MutMutable]; - let mut it = elements - .iter_mut() - .filter_map(|&mut m| { + [hir::MutImmutable, hir::MutMutable] + .iter() + .filter_map(|&m| { let autoref_ty = tcx.mk_ref(region, ty::TypeAndMut { ty: step.self_ty, mutbl: m, }); self.pick_method(autoref_ty).map(|r| { - r.map(|mut picks| { - for pick in picks.iter_mut() { - pick.autoderefs = step.autoderefs; - pick.autoref = Some(m); - pick.unsize = if step.unsize { - Some(step.self_ty) - } else { - None - }; - } - picks + r.map(|mut pick| { + pick.autoderefs = step.autoderefs; + pick.autoref = Some(m); + pick.unsize = if step.unsize { + Some(step.self_ty) + } else { + None + }; + pick }) }) - }); - match self.looking_for { - LookingFor::MethodName(_) => it.nth(0), - LookingFor::ReturnType(_) => { - let ret = it.filter_map(|entry| entry.ok()) - .flat_map(|v| v) - .collect::>(); - - if ret.len() < 1 { - None - } else { - Some(Ok(ret)) - } - } - } + }) + .nth(0) } fn pick_method(&mut self, self_ty: Ty<'tcx>) -> Option> { @@ -1138,7 +1112,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { if applicable_candidates.len() > 1 { match self.collapse_candidates_to_trait_pick(&applicable_candidates[..]) { Some(pick) => { - return Some(Ok(vec![pick])); + return Some(Ok(pick)); } None => {} } @@ -1149,22 +1123,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { return Some(Err(MethodError::Ambiguity(sources))); } - match self.looking_for { - LookingFor::MethodName(_) => applicable_candidates - .pop() - .map(|probe| Ok(vec![probe.to_unadjusted_pick()])), - LookingFor::ReturnType(_) => { - let ret: Vec<_> = applicable_candidates.iter() - .map(|probe| probe.to_unadjusted_pick()) - .collect(); - - if ret.len() < 1 { - None - } else { - Some(Ok(ret)) - } - } - } + applicable_candidates.pop().map(|probe| Ok(probe.to_unadjusted_pick())) } fn consider_probe(&self,