Skip to content

Suggest collecting into Vec<_> #105523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 14, 2022
Merged

Suggest collecting into Vec<_> #105523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9d5e7d3
Suggest `collect`ing into `Vec<_>`
estebank Dec 10, 2022
b3fba5e
Remove unnecessary code and account for turbofish suggestion
estebank Dec 10, 2022
3e25bcb
Mention implementations that satisfy the trait
estebank Dec 11, 2022
7d1e47a
Suggest `: Type` instead of `: _`
estebank Dec 11, 2022
165efab
review comments
estebank Dec 13, 2022
40a6275
rename argument
estebank Dec 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 66 additions & 38 deletions compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
View file
Open in desktop
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 @@ -1089,6 +1116,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
Open in desktop
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
Open in desktop
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 @@ -1432,6 +1434,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 @@ -1887,7 +1890,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 @@ -1938,7 +1943,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 @@ -2179,14 +2184,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 @@ -2198,60 +2215,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
Open in desktop
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
Open in desktop
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
Open in desktop
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