Skip to content

Commit aba56dd

Browse files
committed
type error method suggestions use whitelisted identity-like conversions
Previously, on a type mismatch (and if this wasn't preëmpted by a higher-priority suggestion), we would look for argumentless methods returning the expected type, and list them in a `help` note. This had two major shortcomings. Firstly, a lot of the suggestions didn't really make sense (if you used a &str where a String was expected, `.to_ascii_uppercase()` is probably not the solution you were hoping for). Secondly, we weren't generating suggestions from the most useful traits! We address the first problem with an internal `#[rustc_conversion_suggestion]` attribute meant to mark methods that keep the "same value" in the relevant sense, just converting the type. We address the second problem by making `FnCtxt.probe_for_return_type` pass the `ProbeScope::AllTraits` to `probe_op`: this would seem to be safe because grep reveals no other callers of `probe_for_return_type`. Also, structured suggestions are preferred (because they're pretty, but also for RLS and friends). Also also, we make the E0055 autoderef recursion limit error use the one-time-diagnostics set, because we can potentially hit the limit a lot during probing. (Without this, test/ui/did_you_mean/recursion_limit_deref.rs would report "aborting due to 51 errors"). Unfortunately, the trait probing is still not all one would hope for: at a minimum, we don't know how to rule out `into()` in cases where it wouldn't actually work, and we don't know how to rule in `.to_owned()` where it would. Issues #46459 and #46460 have been filed and are ref'd in a FIXME. This is hoped to resolve #42929, #44672, and #45777.
1 parent 72176cf commit aba56dd

File tree

14 files changed

+147
-65
lines changed

14 files changed

+147
-65
lines changed

src/liballoc/slice.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1595,6 +1595,7 @@ impl<T> [T] {
15951595
/// let x = s.to_vec();
15961596
/// // Here, `s` and `x` can be modified independently.
15971597
/// ```
1598+
#[rustc_conversion_suggestion]
15981599
#[stable(feature = "rust1", since = "1.0.0")]
15991600
#[inline]
16001601
pub fn to_vec(&self) -> Vec<T>

src/liballoc/string.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2034,6 +2034,7 @@ pub trait ToString {
20342034
///
20352035
/// assert_eq!(five, i.to_string());
20362036
/// ```
2037+
#[rustc_conversion_suggestion]
20372038
#[stable(feature = "rust1", since = "1.0.0")]
20382039
fn to_string(&self) -> String;
20392040
}

src/librustc_typeck/check/autoderef.rs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use super::{FnCtxt, LvalueOp};
1414
use super::method::MethodCallee;
1515

1616
use rustc::infer::InferOk;
17+
use rustc::session::DiagnosticMessageId;
1718
use rustc::traits;
1819
use rustc::ty::{self, Ty, TraitRef};
1920
use rustc::ty::{ToPredicate, TypeFoldable};
@@ -56,19 +57,25 @@ impl<'a, 'gcx, 'tcx> Iterator for Autoderef<'a, 'gcx, 'tcx> {
5657
return Some((self.cur_ty, 0));
5758
}
5859

59-
if self.steps.len() == tcx.sess.recursion_limit.get() {
60+
if self.steps.len() >= tcx.sess.recursion_limit.get() {
6061
// We've reached the recursion limit, error gracefully.
6162
let suggested_limit = tcx.sess.recursion_limit.get() * 2;
62-
struct_span_err!(tcx.sess,
63-
self.span,
64-
E0055,
65-
"reached the recursion limit while auto-dereferencing {:?}",
66-
self.cur_ty)
67-
.span_label(self.span, "deref recursion limit reached")
68-
.help(&format!(
69-
"consider adding a `#[recursion_limit=\"{}\"]` attribute to your crate",
63+
let msg = format!("reached the recursion limit while auto-dereferencing {:?}",
64+
self.cur_ty);
65+
let error_id = (DiagnosticMessageId::ErrorId(55), Some(self.span), msg.clone());
66+
let fresh = tcx.sess.one_time_diagnostics.borrow_mut().insert(error_id);
67+
if fresh {
68+
struct_span_err!(tcx.sess,
69+
self.span,
70+
E0055,
71+
"reached the recursion limit while auto-dereferencing {:?}",
72+
self.cur_ty)
73+
.span_label(self.span, "deref recursion limit reached")
74+
.help(&format!(
75+
"consider adding a `#![recursion_limit=\"{}\"]` attribute to your crate",
7076
suggested_limit))
71-
.emit();
77+
.emit();
78+
}
7279
return None;
7380
}
7481

src/librustc_typeck/check/demand.rs

Lines changed: 34 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11+
use std::iter;
1112

1213
use check::FnCtxt;
1314
use rustc::infer::InferOk;
@@ -137,49 +138,45 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
137138
if let Some((msg, suggestion)) = self.check_ref(expr, checked_ty, expected) {
138139
err.span_suggestion(expr.span, msg, suggestion);
139140
} else {
140-
let mode = probe::Mode::MethodCall;
141-
let suggestions = self.probe_for_return_type(syntax_pos::DUMMY_SP,
142-
mode,
143-
expected,
144-
checked_ty,
145-
ast::DUMMY_NODE_ID);
146-
if suggestions.len() > 0 {
147-
err.help(&format!("here are some functions which \
148-
might fulfill your needs:\n{}",
149-
self.get_best_match(&suggestions).join("\n")));
141+
let methods = self.get_conversion_methods(expected, checked_ty);
142+
if let Ok(expr_text) = self.tcx.sess.codemap().span_to_snippet(expr.span) {
143+
let suggestions = iter::repeat(expr_text).zip(methods.iter())
144+
.map(|(receiver, method)| format!("{}.{}()", receiver, method.name))
145+
.collect::<Vec<_>>();
146+
if !suggestions.is_empty() {
147+
err.span_suggestions(expr.span,
148+
"try using a conversion method",
149+
suggestions);
150+
}
150151
}
151152
}
152153
(expected, Some(err))
153154
}
154155

155-
fn format_method_suggestion(&self, method: &AssociatedItem) -> String {
156-
format!("- .{}({})",
157-
method.name,
158-
if self.has_no_input_arg(method) {
159-
""
160-
} else {
161-
"..."
162-
})
163-
}
164-
165-
fn display_suggested_methods(&self, methods: &[AssociatedItem]) -> Vec<String> {
166-
methods.iter()
167-
.take(5)
168-
.map(|method| self.format_method_suggestion(&*method))
169-
.collect::<Vec<String>>()
170-
}
156+
fn get_conversion_methods(&self, expected: Ty<'tcx>, checked_ty: Ty<'tcx>)
157+
-> Vec<AssociatedItem> {
158+
let mut methods = self.probe_for_return_type(syntax_pos::DUMMY_SP,
159+
probe::Mode::MethodCall,
160+
expected,
161+
checked_ty,
162+
ast::DUMMY_NODE_ID);
163+
methods.retain(|m| {
164+
self.has_no_input_arg(m) &&
165+
self.tcx.get_attrs(m.def_id).iter()
166+
// This special internal attribute is used to whitelist
167+
// "identity-like" conversion methods to be suggested here.
168+
//
169+
// FIXME (#46459 and #46460): ideally
170+
// `std::convert::Into::into` and `std::borrow:ToOwned` would
171+
// also be `#[rustc_conversion_suggestion]`, if not for
172+
// method-probing false-positives and -negatives (respectively).
173+
//
174+
// FIXME? Other potential candidate methods: `as_ref` and
175+
// `as_mut`?
176+
.find(|a| a.check_name("rustc_conversion_suggestion")).is_some()
177+
});
171178

172-
fn get_best_match(&self, methods: &[AssociatedItem]) -> Vec<String> {
173-
let no_argument_methods: Vec<_> =
174-
methods.iter()
175-
.filter(|ref x| self.has_no_input_arg(&*x))
176-
.map(|x| x.clone())
177-
.collect();
178-
if no_argument_methods.len() > 0 {
179-
self.display_suggested_methods(&no_argument_methods)
180-
} else {
181-
self.display_suggested_methods(&methods)
182-
}
179+
methods
183180
}
184181

185182
// This function checks if the method isn't static and takes other arguments than `self`.

src/librustc_typeck/check/method/probe.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
190190
scope_expr_id);
191191
let method_names =
192192
self.probe_op(span, mode, None, Some(return_type), IsSuggestion(true),
193-
self_ty, scope_expr_id, ProbeScope::TraitsInScope,
193+
self_ty, scope_expr_id, ProbeScope::AllTraits,
194194
|probe_cx| Ok(probe_cx.candidate_method_names()))
195195
.unwrap_or(vec![]);
196196
method_names
@@ -199,7 +199,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
199199
self.probe_op(
200200
span, mode, Some(method_name), Some(return_type),
201201
IsSuggestion(true), self_ty, scope_expr_id,
202-
ProbeScope::TraitsInScope, |probe_cx| probe_cx.pick()
202+
ProbeScope::AllTraits, |probe_cx| probe_cx.pick()
203203
).ok().map(|pick| pick.item)
204204
})
205205
.collect()

src/libstd/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@
229229

230230
// Turn warnings into errors, but only after stage0, where it can be useful for
231231
// code to emit warnings during language transitions
232-
#![deny(warnings)]
232+
#![cfg_attr(not(stage0), deny(warnings))]
233233

234234
// std may use features in a platform-specific way
235235
#![allow(unused_features)]

src/libstd/path.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1702,6 +1702,7 @@ impl Path {
17021702
/// let path_buf = Path::new("foo.txt").to_path_buf();
17031703
/// assert_eq!(path_buf, std::path::PathBuf::from("foo.txt"));
17041704
/// ```
1705+
#[rustc_conversion_suggestion]
17051706
#[stable(feature = "rust1", since = "1.0.0")]
17061707
pub fn to_path_buf(&self) -> PathBuf {
17071708
PathBuf::from(self.inner.to_os_string())

src/libsyntax/feature_gate.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,13 @@ pub const BUILTIN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeG
966966
never be stable",
967967
cfg_fn!(rustc_attrs))),
968968

969+
// whitelists "identity-like" conversion methods to suggest on type mismatch
970+
("rustc_conversion_suggestion", Whitelisted, Gated(Stability::Unstable,
971+
"rustc_attrs",
972+
"this is an internal attribute that will \
973+
never be stable",
974+
cfg_fn!(rustc_attrs))),
975+
969976
("wasm_import_memory", Whitelisted, Gated(Stability::Unstable,
970977
"wasm_import_memory",
971978
"wasm_import_memory attribute is currently unstable",

src/test/ui/deref-suggestion.stderr

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,13 @@ error[E0308]: mismatched types
22
--> $DIR/deref-suggestion.rs:18:9
33
|
44
18 | foo(s); //~ ERROR mismatched types
5-
| ^ expected struct `std::string::String`, found reference
5+
| ^
6+
| |
7+
| expected struct `std::string::String`, found reference
8+
| help: try using a conversion method: `s.to_string()`
69
|
710
= note: expected type `std::string::String`
811
found type `&std::string::String`
9-
= help: here are some functions which might fulfill your needs:
10-
- .escape_debug()
11-
- .escape_default()
12-
- .escape_unicode()
13-
- .to_ascii_lowercase()
14-
- .to_ascii_uppercase()
1512

1613
error[E0308]: mismatched types
1714
--> $DIR/deref-suggestion.rs:23:10

src/test/ui/did_you_mean/recursion_limit_deref.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ error[E0055]: reached the recursion limit while auto-dereferencing I
44
62 | let x: &Bottom = &t; //~ ERROR mismatched types
55
| ^^ deref recursion limit reached
66
|
7-
= help: consider adding a `#[recursion_limit="20"]` attribute to your crate
7+
= help: consider adding a `#![recursion_limit="20"]` attribute to your crate
88

99
error[E0055]: reached the recursion limit while auto-dereferencing I
1010
|
11-
= help: consider adding a `#[recursion_limit="20"]` attribute to your crate
11+
= help: consider adding a `#![recursion_limit="20"]` attribute to your crate
1212

1313
error[E0308]: mismatched types
1414
--> $DIR/recursion_limit_deref.rs:62:22

0 commit comments

Comments
 (0)