Skip to content

Commit

Permalink
Rollup merge of #105523 - estebank:suggest-collect-vec, r=compiler-er…
Browse files Browse the repository at this point in the history 
…rors

Suggest `collect`ing into `Vec<_>`

Fix #105510.
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Dec 14, 2022
2 parents c8fd654 + 40a6275 commit e5fde96
Show file tree
Hide file tree
Showing 31 changed files with 168 additions and 151 deletions.
104 changes: 66 additions & 38 deletions compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_middle::ty::print::{FmtPrinter, PrettyPrinter, Print, Printer};
use rustc_middle::ty::{self, DefIdTree, InferConst};
use rustc_middle::ty::{GenericArg, GenericArgKind, SubstsRef};
use rustc_middle::ty::{IsSuggestable, Ty, TyCtxt, TypeckResults};
use rustc_span::symbol::{kw, Ident};
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::{BytePos, Span};
use std::borrow::Cow;
use std::iter;
Expand Down Expand Up @@ -79,7 +79,7 @@ impl InferenceDiagnosticsData {

fn where_x_is_kind(&self, in_type: Ty<'_>) -> &'static str {
if in_type.is_ty_infer() {
"empty"
""
} else if self.name == "_" {
// FIXME: Consider specializing this message if there is a single `_`
// in the type.
Expand Down Expand Up @@ -183,13 +183,24 @@ fn fmt_printer<'a, 'tcx>(infcx: &'a InferCtxt<'tcx>, ns: Namespace) -> FmtPrinte
printer
}

fn ty_to_string<'tcx>(infcx: &InferCtxt<'tcx>, ty: Ty<'tcx>) -> String {
fn ty_to_string<'tcx>(
infcx: &InferCtxt<'tcx>,
ty: Ty<'tcx>,
called_method_def_id: Option<DefId>,
) -> String {
let printer = fmt_printer(infcx, Namespace::TypeNS);
let ty = infcx.resolve_vars_if_possible(ty);
match ty.kind() {
match (ty.kind(), called_method_def_id) {
// We don't want the regular output for `fn`s because it includes its path in
// invalid pseudo-syntax, we want the `fn`-pointer output instead.
ty::FnDef(..) => ty.fn_sig(infcx.tcx).print(printer).unwrap().into_buffer(),
(ty::FnDef(..), _) => ty.fn_sig(infcx.tcx).print(printer).unwrap().into_buffer(),
(_, Some(def_id))
if ty.is_ty_infer()
&& infcx.tcx.get_diagnostic_item(sym::iterator_collect_fn) == Some(def_id) =>
{
"Vec<_>".to_string()
}
_ if ty.is_ty_infer() => "/* Type */".to_string(),
// FIXME: The same thing for closures, but this only works when the closure
// does not capture anything.
//
Expand All @@ -213,15 +224,15 @@ fn closure_as_fn_str<'tcx>(infcx: &InferCtxt<'tcx>, ty: Ty<'tcx>) -> String {
.map(|args| {
args.tuple_fields()
.iter()
.map(|arg| ty_to_string(infcx, arg))
.map(|arg| ty_to_string(infcx, arg, None))
.collect::<Vec<_>>()
.join(", ")
})
.unwrap_or_default();
let ret = if fn_sig.output().skip_binder().is_unit() {
String::new()
} else {
format!(" -> {}", ty_to_string(infcx, fn_sig.output().skip_binder()))
format!(" -> {}", ty_to_string(infcx, fn_sig.output().skip_binder(), None))
};
format!("fn({}){}", args, ret)
}
Expand Down Expand Up @@ -368,6 +379,7 @@ impl<'tcx> InferCtxt<'tcx> {
}

impl<'tcx> TypeErrCtxt<'_, 'tcx> {
#[instrument(level = "debug", skip(self, error_code))]
pub fn emit_inference_failure_err(
&self,
body_id: Option<hir::BodyId>,
Expand Down Expand Up @@ -406,7 +418,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
let mut infer_subdiags = Vec::new();
let mut multi_suggestions = Vec::new();
match kind {
InferSourceKind::LetBinding { insert_span, pattern_name, ty } => {
InferSourceKind::LetBinding { insert_span, pattern_name, ty, def_id } => {
infer_subdiags.push(SourceKindSubdiag::LetLike {
span: insert_span,
name: pattern_name.map(|name| name.to_string()).unwrap_or_else(String::new),
Expand All @@ -415,7 +427,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
prefix: arg_data.kind.try_get_prefix().unwrap_or_default(),
arg_name: arg_data.name,
kind: if pattern_name.is_some() { "with_pattern" } else { "other" },
type_name: ty_to_string(self, ty),
type_name: ty_to_string(self, ty, def_id),
});
}
InferSourceKind::ClosureArg { insert_span, ty } => {
Expand All @@ -427,7 +439,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
prefix: arg_data.kind.try_get_prefix().unwrap_or_default(),
arg_name: arg_data.name,
kind: "closure",
type_name: ty_to_string(self, ty),
type_name: ty_to_string(self, ty, None),
});
}
InferSourceKind::GenericArg {
Expand Down Expand Up @@ -456,33 +468,39 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
parent_name,
});

let args = fmt_printer(self, Namespace::TypeNS)
.comma_sep(generic_args.iter().copied().map(|arg| {
if arg.is_suggestable(self.tcx, true) {
return arg;
}
let args = if self.infcx.tcx.get_diagnostic_item(sym::iterator_collect_fn)
== Some(generics_def_id)
{
"Vec<_>".to_string()
} else {
fmt_printer(self, Namespace::TypeNS)
.comma_sep(generic_args.iter().copied().map(|arg| {
if arg.is_suggestable(self.tcx, true) {
return arg;
}

match arg.unpack() {
GenericArgKind::Lifetime(_) => bug!("unexpected lifetime"),
GenericArgKind::Type(_) => self
.next_ty_var(TypeVariableOrigin {
span: rustc_span::DUMMY_SP,
kind: TypeVariableOriginKind::MiscVariable,
})
.into(),
GenericArgKind::Const(arg) => self
.next_const_var(
arg.ty(),
ConstVariableOrigin {
match arg.unpack() {
GenericArgKind::Lifetime(_) => bug!("unexpected lifetime"),
GenericArgKind::Type(_) => self
.next_ty_var(TypeVariableOrigin {
span: rustc_span::DUMMY_SP,
kind: ConstVariableOriginKind::MiscVariable,
},
)
.into(),
}
}))
.unwrap()
.into_buffer();
kind: TypeVariableOriginKind::MiscVariable,
})
.into(),
GenericArgKind::Const(arg) => self
.next_const_var(
arg.ty(),
ConstVariableOrigin {
span: rustc_span::DUMMY_SP,
kind: ConstVariableOriginKind::MiscVariable,
},
)
.into(),
}
}))
.unwrap()
.into_buffer()
};

if !have_turbofish {
infer_subdiags.push(SourceKindSubdiag::GenericSuggestion {
Expand Down Expand Up @@ -520,7 +538,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
));
}
InferSourceKind::ClosureReturn { ty, data, should_wrap_expr } => {
let ty_info = ty_to_string(self, ty);
let ty_info = ty_to_string(self, ty, None);
multi_suggestions.push(SourceKindMultiSuggestion::new_closure_return(
ty_info,
data,
Expand Down Expand Up @@ -608,6 +626,7 @@ enum InferSourceKind<'tcx> {
insert_span: Span,
pattern_name: Option<Ident>,
ty: Ty<'tcx>,
def_id: Option<DefId>,
},
ClosureArg {
insert_span: Span,
Expand Down Expand Up @@ -662,7 +681,7 @@ impl<'tcx> InferSourceKind<'tcx> {
if ty.is_closure() {
("closure", closure_as_fn_str(infcx, ty))
} else if !ty.is_ty_infer() {
("normal", ty_to_string(infcx, ty))
("normal", ty_to_string(infcx, ty, None))
} else {
("other", String::new())
}
Expand Down Expand Up @@ -788,10 +807,18 @@ impl<'a, 'tcx> FindInferSourceVisitor<'a, 'tcx> {
/// Uses `fn source_cost` to determine whether this inference source is preferable to
/// previous sources. We generally prefer earlier sources.
#[instrument(level = "debug", skip(self))]
fn update_infer_source(&mut self, new_source: InferSource<'tcx>) {
fn update_infer_source(&mut self, mut new_source: InferSource<'tcx>) {
let cost = self.source_cost(&new_source) + self.attempt;
debug!(?cost);
self.attempt += 1;
if let Some(InferSource { kind: InferSourceKind::GenericArg { def_id: did, ..}, .. }) = self.infer_source
&& let InferSourceKind::LetBinding { ref ty, ref mut def_id, ..} = new_source.kind
&& ty.is_ty_infer()
{
// Customize the output so we talk about `let x: Vec<_> = iter.collect();` instead of
// `let x: _ = iter.collect();`, as this is a very common case.
*def_id = Some(did);
}
if cost < self.infer_source_cost {
self.infer_source_cost = cost;
self.infer_source = Some(new_source);
Expand Down Expand Up @@ -1092,6 +1119,7 @@ impl<'a, 'tcx> Visitor<'tcx> for FindInferSourceVisitor<'a, 'tcx> {
insert_span: local.pat.span.shrink_to_hi(),
pattern_name: local.pat.simple_ident(),
ty,
def_id: None,
},
})
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,7 @@ symbols! {
item_like_imports,
iter,
iter_repeat,
iterator_collect_fn,
kcfi,
keyword,
kind,
Expand Down
79 changes: 23 additions & 56 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use rustc_middle::ty::{
};
use rustc_session::Limit;
use rustc_span::def_id::LOCAL_CRATE;
use rustc_span::symbol::{kw, sym};
use rustc_span::symbol::sym;
use rustc_span::{ExpnKind, Span, DUMMY_SP};
use std::fmt;
use std::iter;
Expand Down Expand Up @@ -980,6 +980,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
trait_ref,
obligation.cause.body_id,
&mut err,
true,
) {
// This is *almost* equivalent to
// `obligation.cause.code().peel_derives()`, but it gives us the
Expand Down Expand Up @@ -1015,6 +1016,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
trait_ref,
obligation.cause.body_id,
&mut err,
true,
);
}
}
Expand Down Expand Up @@ -1434,6 +1436,7 @@ trait InferCtxtPrivExt<'tcx> {
trait_ref: ty::PolyTraitRef<'tcx>,
body_id: hir::HirId,
err: &mut Diagnostic,
other: bool,
) -> bool;

/// Gets the parent trait chain start
Expand Down Expand Up @@ -1888,7 +1891,9 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
trait_ref: ty::PolyTraitRef<'tcx>,
body_id: hir::HirId,
err: &mut Diagnostic,
other: bool,
) -> bool {
let other = if other { "other " } else { "" };
let report = |mut candidates: Vec<TraitRef<'tcx>>, err: &mut Diagnostic| {
candidates.sort();
candidates.dedup();
Expand Down Expand Up @@ -1939,7 +1944,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
candidates.dedup();
let end = if candidates.len() <= 9 { candidates.len() } else { 8 };
err.help(&format!(
"the following other types implement trait `{}`:{}{}",
"the following {other}types implement trait `{}`:{}{}",
trait_ref.print_only_trait_path(),
candidates[..end].join(""),
if len > 9 { format!("\nand {} others", len - 8) } else { String::new() }
Expand Down Expand Up @@ -2180,14 +2185,26 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
trait_ref.skip_binder().substs.types().any(|t| !t.is_ty_infer());
// It doesn't make sense to talk about applicable impls if there are more
// than a handful of them.
if impls.len() > 1 && impls.len() < 5 && has_non_region_infer {
if impls.len() > 1 && impls.len() < 10 && has_non_region_infer {
self.annotate_source_of_ambiguity(&mut err, &impls, predicate);
} else {
if self.tainted_by_errors().is_some() {
err.cancel();
return;
}
err.note(&format!("cannot satisfy `{}`", predicate));
let impl_candidates = self.find_similar_impl_candidates(
predicate.to_opt_poly_trait_pred().unwrap(),
);
if impl_candidates.len() < 10 {
self.report_similar_impl_candidates(
impl_candidates,
trait_ref,
body_id.map(|id| id.hir_id).unwrap_or(obligation.cause.body_id),
&mut err,
false,
);
}
}
}
_ => {
Expand All @@ -2199,60 +2216,10 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
}
}

if let ObligationCauseCode::ItemObligation(def_id) | ObligationCauseCode::ExprItemObligation(def_id, ..) = *obligation.cause.code() {
self.suggest_fully_qualified_path(&mut err, def_id, span, trait_ref.def_id());
} else if let Ok(snippet) = &self.tcx.sess.source_map().span_to_snippet(span)
&& let ObligationCauseCode::BindingObligation(def_id, _) | ObligationCauseCode::ExprBindingObligation(def_id, ..)
= *obligation.cause.code()
if let ObligationCauseCode::ItemObligation(def_id)
| ObligationCauseCode::ExprItemObligation(def_id, ..) = *obligation.cause.code()
{
let generics = self.tcx.generics_of(def_id);
if generics.params.iter().any(|p| p.name != kw::SelfUpper)
&& !snippet.ends_with('>')
&& !generics.has_impl_trait()
&& !self.tcx.is_fn_trait(def_id)
{
// FIXME: To avoid spurious suggestions in functions where type arguments
// where already supplied, we check the snippet to make sure it doesn't
// end with a turbofish. Ideally we would have access to a `PathSegment`
// instead. Otherwise we would produce the following output:
//
// error[E0283]: type annotations needed
// --> $DIR/issue-54954.rs:3:24
// |
// LL | const ARR_LEN: usize = Tt::const_val::<[i8; 123]>();
// | ^^^^^^^^^^^^^^^^^^^^^^^^^^
// | |
// | cannot infer type
// | help: consider specifying the type argument
// | in the function call:
// | `Tt::const_val::<[i8; 123]>::<T>`
// ...
// LL | const fn const_val<T: Sized>() -> usize {
// | - required by this bound in `Tt::const_val`
// |
// = note: cannot satisfy `_: Tt`

// Clear any more general suggestions in favor of our specific one
err.clear_suggestions();

err.span_suggestion_verbose(
span.shrink_to_hi(),
&format!(
"consider specifying the type argument{} in the function call",
pluralize!(generics.params.len()),
),
format!(
"::<{}>",
generics
.params
.iter()
.map(|p| p.name.to_string())
.collect::<Vec<String>>()
.join(", ")
),
Applicability::HasPlaceholders,
);
}
self.suggest_fully_qualified_path(&mut err, def_id, span, trait_ref.def_id());
}

if let (Some(body_id), Some(ty::subst::GenericArgKind::Type(_))) =
Expand Down
1 change: 1 addition & 0 deletions library/core/src/iter/traits/iterator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1829,6 +1829,7 @@ pub trait Iterator {
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[must_use = "if you really need to exhaust the iterator, consider `.for_each(drop)` instead"]
#[cfg_attr(not(test), rustc_diagnostic_item = "iterator_collect_fn")]
fn collect<B: FromIterator<Self::Item>>(self) -> B
where
Self: Sized,
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/array-slice-vec/infer_array_len.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ LL | let [_, _] = a.into();
|
help: consider giving this pattern a type
|
LL | let [_, _]: _ = a.into();
| +++
LL | let [_, _]: /* Type */ = a.into();
| ++++++++++++

error: aborting due to previous error

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/closure-expected-type/expect-two-infer-vars-supply-ty-with-bound-region.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ LL | with_closure(|x: u32, y| {});
|
help: consider giving this closure parameter an explicit type
|
LL | with_closure(|x: u32, y: _| {});
| +++
LL | with_closure(|x: u32, y: /* Type */| {});
| ++++++++++++

error: aborting due to previous error

Expand Down
Loading

0 comments on commit e5fde96

Please sign in to comment.