Skip to content

Commit

Permalink
type error method suggestions use whitelisted identity-like conversions
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
zackmdavis committed Jan 7, 2018
1 parent 72176cf commit aba56dd
Show file tree
Hide file tree
Showing 14 changed files with 147 additions and 65 deletions.
1 change: 1 addition & 0 deletions src/liballoc/slice.rs
Expand Up @@ -1595,6 +1595,7 @@ impl<T> [T] {
/// let x = s.to_vec();
/// // Here, `s` and `x` can be modified independently.
/// ```
#[rustc_conversion_suggestion]
#[stable(feature = "rust1", since = "1.0.0")]
#[inline]
pub fn to_vec(&self) -> Vec<T>
Expand Down
1 change: 1 addition & 0 deletions src/liballoc/string.rs
Expand Up @@ -2034,6 +2034,7 @@ pub trait ToString {
///
/// assert_eq!(five, i.to_string());
/// ```
#[rustc_conversion_suggestion]
#[stable(feature = "rust1", since = "1.0.0")]
fn to_string(&self) -> String;
}
Expand Down
27 changes: 17 additions & 10 deletions src/librustc_typeck/check/autoderef.rs
Expand Up @@ -14,6 +14,7 @@ use super::{FnCtxt, LvalueOp};
use super::method::MethodCallee;

use rustc::infer::InferOk;
use rustc::session::DiagnosticMessageId;
use rustc::traits;
use rustc::ty::{self, Ty, TraitRef};
use rustc::ty::{ToPredicate, TypeFoldable};
Expand Down Expand Up @@ -56,19 +57,25 @@ impl<'a, 'gcx, 'tcx> Iterator for Autoderef<'a, 'gcx, 'tcx> {
return Some((self.cur_ty, 0));
}

if self.steps.len() == tcx.sess.recursion_limit.get() {
if self.steps.len() >= tcx.sess.recursion_limit.get() {
// We've reached the recursion limit, error gracefully.
let suggested_limit = tcx.sess.recursion_limit.get() * 2;
struct_span_err!(tcx.sess,
self.span,
E0055,
"reached the recursion limit while auto-dereferencing {:?}",
self.cur_ty)
.span_label(self.span, "deref recursion limit reached")
.help(&format!(
"consider adding a `#[recursion_limit=\"{}\"]` attribute to your crate",
let msg = format!("reached the recursion limit while auto-dereferencing {:?}",
self.cur_ty);
let error_id = (DiagnosticMessageId::ErrorId(55), Some(self.span), msg.clone());
let fresh = tcx.sess.one_time_diagnostics.borrow_mut().insert(error_id);
if fresh {
struct_span_err!(tcx.sess,
self.span,
E0055,
"reached the recursion limit while auto-dereferencing {:?}",
self.cur_ty)
.span_label(self.span, "deref recursion limit reached")
.help(&format!(
"consider adding a `#![recursion_limit=\"{}\"]` attribute to your crate",
suggested_limit))
.emit();
.emit();
}
return None;
}

Expand Down
71 changes: 34 additions & 37 deletions src/librustc_typeck/check/demand.rs
Expand Up @@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::iter;

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

fn format_method_suggestion(&self, method: &AssociatedItem) -> String {
format!("- .{}({})",
method.name,
if self.has_no_input_arg(method) {
""
} else {
"..."
})
}

fn display_suggested_methods(&self, methods: &[AssociatedItem]) -> Vec<String> {
methods.iter()
.take(5)
.map(|method| self.format_method_suggestion(&*method))
.collect::<Vec<String>>()
}
fn get_conversion_methods(&self, expected: Ty<'tcx>, checked_ty: Ty<'tcx>)
-> Vec<AssociatedItem> {
let mut methods = self.probe_for_return_type(syntax_pos::DUMMY_SP,
probe::Mode::MethodCall,
expected,
checked_ty,
ast::DUMMY_NODE_ID);
methods.retain(|m| {
self.has_no_input_arg(m) &&
self.tcx.get_attrs(m.def_id).iter()
// This special internal attribute is used to whitelist
// "identity-like" conversion methods to be suggested here.
//
// FIXME (#46459 and #46460): ideally
// `std::convert::Into::into` and `std::borrow:ToOwned` would
// also be `#[rustc_conversion_suggestion]`, if not for
// method-probing false-positives and -negatives (respectively).
//
// FIXME? Other potential candidate methods: `as_ref` and
// `as_mut`?
.find(|a| a.check_name("rustc_conversion_suggestion")).is_some()
});

fn get_best_match(&self, methods: &[AssociatedItem]) -> Vec<String> {
let no_argument_methods: Vec<_> =
methods.iter()
.filter(|ref x| self.has_no_input_arg(&*x))
.map(|x| x.clone())
.collect();
if no_argument_methods.len() > 0 {
self.display_suggested_methods(&no_argument_methods)
} else {
self.display_suggested_methods(&methods)
}
methods
}

// This function checks if the method isn't static and takes other arguments than `self`.
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_typeck/check/method/probe.rs
Expand Up @@ -190,7 +190,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
scope_expr_id);
let method_names =
self.probe_op(span, mode, None, Some(return_type), IsSuggestion(true),
self_ty, scope_expr_id, ProbeScope::TraitsInScope,
self_ty, scope_expr_id, ProbeScope::AllTraits,
|probe_cx| Ok(probe_cx.candidate_method_names()))
.unwrap_or(vec![]);
method_names
Expand All @@ -199,7 +199,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
self.probe_op(
span, mode, Some(method_name), Some(return_type),
IsSuggestion(true), self_ty, scope_expr_id,
ProbeScope::TraitsInScope, |probe_cx| probe_cx.pick()
ProbeScope::AllTraits, |probe_cx| probe_cx.pick()
).ok().map(|pick| pick.item)
})
.collect()
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/lib.rs
Expand Up @@ -229,7 +229,7 @@

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

// std may use features in a platform-specific way
#![allow(unused_features)]
Expand Down
1 change: 1 addition & 0 deletions src/libstd/path.rs
Expand Up @@ -1702,6 +1702,7 @@ impl Path {
/// let path_buf = Path::new("foo.txt").to_path_buf();
/// assert_eq!(path_buf, std::path::PathBuf::from("foo.txt"));
/// ```
#[rustc_conversion_suggestion]
#[stable(feature = "rust1", since = "1.0.0")]
pub fn to_path_buf(&self) -> PathBuf {
PathBuf::from(self.inner.to_os_string())
Expand Down
7 changes: 7 additions & 0 deletions src/libsyntax/feature_gate.rs
Expand Up @@ -966,6 +966,13 @@ pub const BUILTIN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeG
never be stable",
cfg_fn!(rustc_attrs))),

// whitelists "identity-like" conversion methods to suggest on type mismatch
("rustc_conversion_suggestion", Whitelisted, Gated(Stability::Unstable,
"rustc_attrs",
"this is an internal attribute that will \
never be stable",
cfg_fn!(rustc_attrs))),

("wasm_import_memory", Whitelisted, Gated(Stability::Unstable,
"wasm_import_memory",
"wasm_import_memory attribute is currently unstable",
Expand Down
11 changes: 4 additions & 7 deletions src/test/ui/deref-suggestion.stderr
Expand Up @@ -2,16 +2,13 @@ error[E0308]: mismatched types
--> $DIR/deref-suggestion.rs:18:9
|
18 | foo(s); //~ ERROR mismatched types
| ^ expected struct `std::string::String`, found reference
| ^
| |
| expected struct `std::string::String`, found reference
| help: try using a conversion method: `s.to_string()`
|
= note: expected type `std::string::String`
found type `&std::string::String`
= help: here are some functions which might fulfill your needs:
- .escape_debug()
- .escape_default()
- .escape_unicode()
- .to_ascii_lowercase()
- .to_ascii_uppercase()

error[E0308]: mismatched types
--> $DIR/deref-suggestion.rs:23:10
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/did_you_mean/recursion_limit_deref.stderr
Expand Up @@ -4,11 +4,11 @@ error[E0055]: reached the recursion limit while auto-dereferencing I
62 | let x: &Bottom = &t; //~ ERROR mismatched types
| ^^ deref recursion limit reached
|
= help: consider adding a `#[recursion_limit="20"]` attribute to your crate
= help: consider adding a `#![recursion_limit="20"]` attribute to your crate

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

error[E0308]: mismatched types
--> $DIR/recursion_limit_deref.rs:62:22
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/span/coerce-suggestions.stderr
Expand Up @@ -6,9 +6,6 @@ error[E0308]: mismatched types
|
= note: expected type `usize`
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:19:19
Expand Down Expand Up @@ -44,7 +41,10 @@ error[E0308]: mismatched types
--> $DIR/coerce-suggestions.rs:27:9
|
27 | f = box f;
| ^^^^^ cyclic type of infinite size
| ^^^^^
| |
| cyclic type of infinite size
| help: try using a conversion method: `box f.to_string()`

error[E0308]: mismatched types
--> $DIR/coerce-suggestions.rs:31:9
Expand Down
2 changes: 0 additions & 2 deletions src/test/ui/span/issue-34264.stderr
Expand Up @@ -33,8 +33,6 @@ error[E0308]: mismatched types
|
= note: expected type `usize`
found type `&'static str`
= help: here are some functions which might fulfill your needs:
- .len()

error[E0061]: this function takes 2 parameters but 3 parameters were supplied
--> $DIR/issue-34264.rs:20:5
Expand Down
23 changes: 23 additions & 0 deletions src/test/ui/suggestions/conversion-methods.rs
@@ -0,0 +1,23 @@
// Copyright 2017 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::path::{Path, PathBuf};


fn main() {
let _tis_an_instants_play: String = "'Tis a fond Ambush—"; //~ ERROR mismatched types
let _just_to_make_bliss: PathBuf = Path::new("/ern/her/own/surprise");
//~^ ERROR mismatched types

let _but_should_the_play: String = 2; // Perhaps surprisingly, we suggest .to_string() here
//~^ ERROR mismatched types

let _prove_piercing_earnest: Vec<usize> = &[1, 2, 3]; //~ ERROR mismatched types
}
50 changes: 50 additions & 0 deletions src/test/ui/suggestions/conversion-methods.stderr
@@ -0,0 +1,50 @@
error[E0308]: mismatched types
--> $DIR/conversion-methods.rs:15:41
|
15 | let _tis_an_instants_play: String = "'Tis a fond Ambush—"; //~ ERROR mismatched types
| ^^^^^^^^^^^^^^^^^^^^^
| |
| expected struct `std::string::String`, found reference
| help: try using a conversion method: `"'Tis a fond Ambush—".to_string()`
|
= note: expected type `std::string::String`
found type `&'static str`

error[E0308]: mismatched types
--> $DIR/conversion-methods.rs:16:40
|
16 | let _just_to_make_bliss: PathBuf = Path::new("/ern/her/own/surprise");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| expected struct `std::path::PathBuf`, found reference
| help: try using a conversion method: `Path::new("/ern/her/own/surprise").to_path_buf()`
|
= note: expected type `std::path::PathBuf`
found type `&std::path::Path`

error[E0308]: mismatched types
--> $DIR/conversion-methods.rs:19:40
|
19 | let _but_should_the_play: String = 2; // Perhaps surprisingly, we suggest .to_string() here
| ^
| |
| expected struct `std::string::String`, found integral variable
| help: try using a conversion method: `2.to_string()`
|
= note: expected type `std::string::String`
found type `{integer}`

error[E0308]: mismatched types
--> $DIR/conversion-methods.rs:22:47
|
22 | let _prove_piercing_earnest: Vec<usize> = &[1, 2, 3]; //~ ERROR mismatched types
| ^^^^^^^^^^
| |
| expected struct `std::vec::Vec`, found reference
| help: try using a conversion method: `&[1, 2, 3].to_vec()`
|
= note: expected type `std::vec::Vec<usize>`
found type `&[{integer}; 3]`

error: aborting due to 4 previous errors

0 comments on commit aba56dd

Please sign in to comment.