Skip to content

Commit a9defa8

Browse files
GuillaumeGomezGuillaumeGomez
committed
splitting methods into smaller ones, add docs, better variable naming
1 parent 82d31c4 commit a9defa8

File tree

3 files changed

+109
-64
lines changed

3 files changed

+109
-64
lines changed

‎src/librustc_typeck/check/demand.rs

+65-28
Original file line numberDiff line numberDiff line change
@@ -58,22 +58,50 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
5858
}
5959
}
6060

61-
fn check_ref(&self, expr: &hir::Expr, checked_ty: Ty<'tcx>,
62-
expected: Ty<'tcx>) -> Option<String> {
63-
match (&checked_ty.sty, &expected.sty) {
64-
(&ty::TyRef(_, x_mutability), &ty::TyRef(_, y_mutability)) => {
61+
/// This function is used to determine potential "simple" improvements or users' errors and
62+
/// provide them useful help. For example:
63+
///
64+
/// ```
65+
/// fn some_fn(s: &str) {}
66+
///
67+
/// let x = "hey!".to_owned();
68+
/// some_fn(x); // error
69+
/// ```
70+
///
71+
/// No need to find every potential function which could make a coercion to transform a
72+
/// `String` into a `&str` since a `&` would do the trick!
73+
///
74+
/// In addition of this check, it also checks between references mutability state. If the
75+
/// expected is mutable but the provided isn't, maybe we could just say "Hey, try with
76+
/// `&mut`!".
77+
fn check_ref(&self,
78+
expr: &hir::Expr,
79+
checked_ty: Ty<'tcx>,
80+
expected: Ty<'tcx>)
81+
-> Option<String> {
82+
match (&expected.sty, &checked_ty.sty) {
83+
(&ty::TyRef(_, expected_mutability),
84+
&ty::TyRef(_, checked_mutability)) => {
6585
// check if there is a mutability difference
66-
if x_mutability.mutbl == hir::Mutability::MutImmutable &&
67-
x_mutability.mutbl != y_mutability.mutbl &&
68-
self.can_sub_types(&x_mutability.ty, y_mutability.ty).is_ok() {
86+
if checked_mutability.mutbl == hir::Mutability::MutImmutable &&
87+
checked_mutability.mutbl != expected_mutability.mutbl &&
88+
self.can_sub_types(&checked_mutability.ty,
89+
expected_mutability.ty).is_ok() {
6990
if let Ok(src) = self.tcx.sess.codemap().span_to_snippet(expr.span) {
7091
return Some(format!("try with `&mut {}`", &src.replace("&", "")));
7192
}
7293
}
7394
None
7495
}
75-
(_, &ty::TyRef(_, mutability)) => {
76-
// check if it can work when put into a ref
96+
(&ty::TyRef(_, mutability), _) => {
97+
// Check if it can work when put into a ref. For example:
98+
//
99+
// ```
100+
// fn bar(x: &mut i32) {}
101+
//
102+
// let x = 0u32;
103+
// bar(&x); // error, expected &mut
104+
// ```
77105
let ref_ty = match mutability.mutbl {
78106
hir::Mutability::MutMutable => self.tcx.mk_mut_ref(
79107
self.tcx.mk_region(ty::ReStatic),
@@ -107,11 +135,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
107135
let mode = probe::Mode::MethodCall;
108136
let suggestions = if let Some(s) = self.check_ref(expr, checked_ty, expected) {
109137
Some(s)
110-
} else if let Ok(methods) = self.probe_return(syntax_pos::DUMMY_SP,
111-
mode,
112-
expected,
113-
checked_ty,
114-
ast::DUMMY_NODE_ID) {
138+
} else if let Ok(methods) = self.probe_for_return_type(syntax_pos::DUMMY_SP,
139+
mode,
140+
expected,
141+
checked_ty,
142+
ast::DUMMY_NODE_ID) {
115143
let suggestions: Vec<_> =
116144
methods.iter()
117145
.map(|ref x| {
@@ -136,29 +164,38 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
136164
}
137165
}
138166

167+
fn format_method_suggestion(&self, method: &ImplOrTraitItem<'tcx>) -> String {
168+
format!(".{}({})",
169+
method.name(),
170+
if self.has_not_input_arg(method) {
171+
""
172+
} else {
173+
"..."
174+
})
175+
}
176+
177+
fn display_suggested_methods(&self, methods: &[Rc<ImplOrTraitItem<'tcx>>]) -> String {
178+
methods.iter()
179+
.take(5)
180+
.map(|method| self.format_method_suggestion(&*method))
181+
.collect::<Vec<String>>()
182+
.join("\n - ")
183+
}
184+
139185
fn get_best_match(&self, methods: &[Rc<ImplOrTraitItem<'tcx>>]) -> String {
140-
if methods.len() == 1 {
141-
return format!(" - {}", methods[0].name());
142-
}
143-
let no_argument_methods: Vec<&Rc<ImplOrTraitItem<'tcx>>> =
186+
let no_argument_methods: Vec<Rc<ImplOrTraitItem<'tcx>>> =
144187
methods.iter()
145188
.filter(|ref x| self.has_not_input_arg(&*x))
189+
.map(|x| x.clone())
146190
.collect();
147191
if no_argument_methods.len() > 0 {
148-
no_argument_methods.iter()
149-
.take(5)
150-
.map(|method| format!(".{}()", method.name()))
151-
.collect::<Vec<String>>()
152-
.join("\n - ")
192+
self.display_suggested_methods(&no_argument_methods)
153193
} else {
154-
methods.iter()
155-
.take(5)
156-
.map(|method| format!(".{}()", method.name()))
157-
.collect::<Vec<String>>()
158-
.join("\n - ")
194+
self.display_suggested_methods(&methods)
159195
}
160196
}
161197

198+
// This function checks if the method isn't static and takes other arguments than `self`.
162199
fn has_not_input_arg(&self, method: &ImplOrTraitItem<'tcx>) -> bool {
163200
match *method {
164201
ImplOrTraitItem::MethodTraitItem(ref x) => {

‎src/librustc_typeck/check/method/mod.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
9191
allow_private: bool)
9292
-> bool {
9393
let mode = probe::Mode::MethodCall;
94-
match self.probe_method(span, mode, method_name, self_ty, call_expr_id) {
94+
match self.probe_for_name(span, mode, method_name, self_ty, call_expr_id) {
9595
Ok(..) => true,
9696
Err(NoMatch(..)) => false,
9797
Err(Ambiguity(..)) => true,
@@ -130,7 +130,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
130130

131131
let mode = probe::Mode::MethodCall;
132132
let self_ty = self.resolve_type_vars_if_possible(&self_ty);
133-
let pick = self.probe_method(span, mode, method_name, self_ty, call_expr.id)?.remove(0);
133+
let pick = self.probe_for_name(span, mode, method_name, self_ty, call_expr.id)?.remove(0);
134134

135135
if let Some(import_id) = pick.import_id {
136136
self.tcx.used_trait_imports.borrow_mut().insert(import_id);
@@ -353,7 +353,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
353353
expr_id: ast::NodeId)
354354
-> Result<Def, MethodError<'tcx>> {
355355
let mode = probe::Mode::Path;
356-
let picks = self.probe_method(span, mode, method_name, self_ty, expr_id)?;
356+
let picks = self.probe_for_name(span, mode, method_name, self_ty, expr_id)?;
357357
let pick = &picks[0];
358358

359359
if let Some(import_id) = pick.import_id {

‎src/librustc_typeck/check/method/probe.rs

+41-33
Original file line numberDiff line numberDiff line change
@@ -148,41 +148,41 @@ pub enum Mode {
148148
}
149149

150150
impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
151-
pub fn probe_return(&self,
152-
span: Span,
153-
mode: Mode,
154-
return_type: Ty<'tcx>,
155-
self_ty: Ty<'tcx>,
156-
scope_expr_id: ast::NodeId)
157-
-> PickResult<'tcx> {
151+
pub fn probe_for_return_type(&self,
152+
span: Span,
153+
mode: Mode,
154+
return_type: Ty<'tcx>,
155+
self_ty: Ty<'tcx>,
156+
scope_expr_id: ast::NodeId)
157+
-> PickResult<'tcx> {
158158
debug!("probe(self_ty={:?}, return_type={}, scope_expr_id={})",
159159
self_ty,
160160
return_type,
161161
scope_expr_id);
162-
self._probe(span, mode, LookingFor::ReturnType(return_type), self_ty, scope_expr_id)
162+
self.probe_for(span, mode, LookingFor::ReturnType(return_type), self_ty, scope_expr_id)
163163
}
164164

165-
pub fn probe_method(&self,
166-
span: Span,
167-
mode: Mode,
168-
item_name: ast::Name,
169-
self_ty: Ty<'tcx>,
170-
scope_expr_id: ast::NodeId)
171-
-> PickResult<'tcx> {
165+
pub fn probe_for_name(&self,
166+
span: Span,
167+
mode: Mode,
168+
item_name: ast::Name,
169+
self_ty: Ty<'tcx>,
170+
scope_expr_id: ast::NodeId)
171+
-> PickResult<'tcx> {
172172
debug!("probe(self_ty={:?}, item_name={}, scope_expr_id={})",
173173
self_ty,
174174
item_name,
175175
scope_expr_id);
176-
self._probe(span, mode, LookingFor::MethodName(item_name), self_ty, scope_expr_id)
176+
self.probe_for(span, mode, LookingFor::MethodName(item_name), self_ty, scope_expr_id)
177177
}
178178

179-
fn _probe(&self,
180-
span: Span,
181-
mode: Mode,
182-
looking_for: LookingFor<'tcx>,
183-
self_ty: Ty<'tcx>,
184-
scope_expr_id: ast::NodeId)
185-
-> PickResult<'tcx> {
179+
fn probe_for(&self,
180+
span: Span,
181+
mode: Mode,
182+
looking_for: LookingFor<'tcx>,
183+
self_ty: Ty<'tcx>,
184+
scope_expr_id: ast::NodeId)
185+
-> PickResult<'tcx> {
186186

187187
// FIXME(#18741) -- right now, creating the steps involves evaluating the
188188
// `*` operator, which registers obligations that then escape into
@@ -263,6 +263,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
263263

264264
let final_ty = match looking_for {
265265
&LookingFor::MethodName(_) => autoderef.unambiguous_final_ty(),
266+
// Since ReturnType case tries to coerce the returned type to the
267+
// expected one, we need all the information!
266268
&LookingFor::ReturnType(_) => self_ty,
267269
};
268270
match final_ty.sty {
@@ -636,10 +638,10 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
636638
match *method {
637639
ty::ImplOrTraitItem::MethodTraitItem(ref x) => {
638640
self.probe(|_| {
639-
let output = self.replace_late_bound_regions_with_fresh_var(
640-
self.span, infer::FnCall, &x.fty.sig.output());
641641
let substs = self.fresh_substs_for_item(self.span, method.def_id());
642-
let output = output.0.subst(self.tcx, substs);
642+
let output = x.fty.sig.output().subst(self.tcx, substs);
643+
let (output, _) = self.replace_late_bound_regions_with_fresh_var(
644+
self.span, infer::FnCall, &output);
643645
self.can_sub_types(output, expected).is_ok()
644646
})
645647
}
@@ -958,10 +960,17 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
958960
let steps = self.steps.clone();
959961

960962
match self.looking_for {
961-
LookingFor::MethodName(_) => steps.iter()
962-
.filter_map(|step| self.pick_step(step))
963-
.next(),
963+
LookingFor::MethodName(_) => {
964+
// find the first step that works
965+
steps.iter()
966+
.filter_map(|step| self.pick_step(step))
967+
.next()
968+
}
964969
LookingFor::ReturnType(_) => {
970+
// Normally, we stop at the first step where we find a positive match.
971+
// But when we are scanning for methods with a suitable return type,
972+
// these methods have distinct names and hence may not shadow one another
973+
// (also, this is just for hints, so precision is less important).
965974
let mut ret = Vec::new();
966975

967976
for step in steps.iter() {
@@ -1058,10 +1067,9 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
10581067
match self.looking_for {
10591068
LookingFor::MethodName(_) => it.nth(0),
10601069
LookingFor::ReturnType(_) => {
1061-
let mut ret = Vec::new();
1062-
it.filter_map(|entry| entry.ok())
1063-
.map(|mut v| { ret.append(&mut v); })
1064-
.all(|_| true);
1070+
let ret = it.filter_map(|entry| entry.ok())
1071+
.flat_map(|v| v)
1072+
.collect::<Vec<_>>();
10651073

10661074
if ret.len() < 1 {
10671075
None

0 commit comments

Comments
 (0)