Skip to content
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.

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

Suggest impl Iterator when possible for _ return type #106172

Merged
merged 3 commits into from
Dec 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
61 changes: 60 additions & 1 deletion compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,18 @@ use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{GenericParamKind, Node};
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::util::{Discr, IntTypeExt};
use rustc_middle::ty::{self, AdtKind, Const, IsSuggestable, ToPredicate, Ty, TyCtxt};
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::Span;
use rustc_target::spec::abi;
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::error_reporting::suggestions::NextTypeParamName;
use rustc_trait_selection::traits::ObligationCtxt;
use std::iter;

mod generics_of;
Expand Down Expand Up @@ -1224,7 +1228,17 @@ fn infer_return_ty_for_fn_sig<'tcx>(
// to prevent the user from getting a papercut while trying to use the unique closure
// syntax (e.g. `[closure@src/lib.rs:2:5: 2:9]`).
diag.help("consider using an `Fn`, `FnMut`, or `FnOnce` trait bound");
diag.note("for more information on `Fn` traits and closure types, see https://doc.rust-lang.org/book/ch13-01-closures.html");
diag.note(
"for more information on `Fn` traits and closure types, see \
https://doc.rust-lang.org/book/ch13-01-closures.html",
);
} else if let Some(i_ty) = suggest_impl_iterator(tcx, ret_ty, ty.span, hir_id, def_id) {
diag.span_suggestion(
ty.span,
"replace with an appropriate return type",
format!("impl Iterator<Item = {}>", i_ty),
Applicability::MachineApplicable,
);
}
diag.emit();

Expand All @@ -1242,6 +1256,51 @@ fn infer_return_ty_for_fn_sig<'tcx>(
}
}

fn suggest_impl_iterator<'tcx>(
tcx: TyCtxt<'tcx>,
ret_ty: Ty<'tcx>,
span: Span,
hir_id: hir::HirId,
def_id: LocalDefId,
) -> Option<Ty<'tcx>> {
let Some(iter_trait) = tcx.get_diagnostic_item(sym::Iterator) else { return None; };
let Some(iterator_item) = tcx.get_diagnostic_item(sym::IteratorItem) else { return None; };
if !tcx
.infer_ctxt()
.build()
.type_implements_trait(iter_trait, [ret_ty], tcx.param_env(def_id))
.must_apply_modulo_regions()
{
return None;
}
let infcx = tcx.infer_ctxt().build();
let ocx = ObligationCtxt::new_in_snapshot(&infcx);
// Find the type of `Iterator::Item`.
let origin = TypeVariableOrigin { kind: TypeVariableOriginKind::TypeInference, span };
let ty_var = infcx.next_ty_var(origin);
let projection = ty::Binder::dummy(ty::PredicateKind::Clause(ty::Clause::Projection(
ty::ProjectionPredicate {
projection_ty: tcx.mk_alias_ty(iterator_item, tcx.mk_substs([ret_ty.into()].iter())),
estebank marked this conversation as resolved.
Show resolved Hide resolved
term: ty_var.into(),
},
)));
// Add `<ret_ty as Iterator>::Item = _` obligation.
ocx.register_obligation(crate::traits::Obligation::misc(
tcx,
span,
hir_id,
tcx.param_env(def_id),
projection,
));
Comment on lines +1280 to +1294
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can just construct a projection type then pass it to ocx.normalize instead, should make the code a lot simpler

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I'll fix this in a follow-up PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried making that change, but I must have done it incorrectly and it didn't work. Let's land this and clean it up soon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't worry about it at all, I got it working on a local branch in any case. I do wonder what was wrong, though.

if ocx.select_where_possible().is_empty()
&& let item_ty = infcx.resolve_vars_if_possible(ty_var)
&& item_ty.is_suggestable(tcx, false)
{
return Some(item_ty);
}
None
}

fn impl_trait_ref(tcx: TyCtxt<'_>, def_id: DefId) -> Option<ty::TraitRef<'_>> {
let icx = ItemCtxt::new(tcx, def_id);
let item = tcx.hir().expect_item(def_id.expect_local());
Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_hir_analysis/src/collect/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use rustc_hir::intravisit;
use rustc_hir::intravisit::Visitor;
use rustc_hir::{HirId, Node};
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::print::with_forced_trimmed_paths;
use rustc_middle::ty::subst::InternalSubsts;
use rustc_middle::ty::util::IntTypeExt;
use rustc_middle::ty::{self, DefIdTree, Ty, TyCtxt, TypeFolder, TypeSuperFoldable, TypeVisitable};
Expand Down Expand Up @@ -907,10 +908,10 @@ fn infer_placeholder_type<'a>(
Applicability::MachineApplicable,
);
} else {
err.span_note(
with_forced_trimmed_paths!(err.span_note(
tcx.hir().body(body_id).value.span,
&format!("however, the inferred type `{}` cannot be named", ty),
);
&format!("however, the inferred type `{ty}` cannot be named"),
));
}
}

Expand All @@ -931,10 +932,10 @@ fn infer_placeholder_type<'a>(
Applicability::MaybeIncorrect,
);
} else {
diag.span_note(
with_forced_trimmed_paths!(diag.span_note(
tcx.hir().body(body_id).value.span,
&format!("however, the inferred type `{}` cannot be named", ty),
);
&format!("however, the inferred type `{ty}` cannot be named"),
));
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ symbols! {
Is,
ItemContext,
Iterator,
IteratorItem,
Layout,
Left,
LinkedList,
Expand Down
1 change: 1 addition & 0 deletions library/core/src/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ fn _assert_is_object_safe(_: &dyn Iterator<Item = ()>) {}
#[must_use = "iterators are lazy and do nothing unless consumed"]
pub trait Iterator {
/// The type of the elements being iterated over.
#[rustc_diagnostic_item = "IteratorItem"]
#[stable(feature = "rust1", since = "1.0.0")]
type Item;

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/suggestions/unnamable-types.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ error[E0121]: the placeholder `_` is not allowed within types on item signatures
LL | const C: _ = || 42;
| ^ not allowed in type signatures
|
note: however, the inferred type `[closure@$DIR/unnamable-types.rs:17:14: 17:16]` cannot be named
note: however, the inferred type `[closure@unnamable-types.rs:17:14]` cannot be named
--> $DIR/unnamable-types.rs:17:14
|
LL | const C: _ = || 42;
Expand All @@ -31,7 +31,7 @@ error: missing type for `const` item
LL | const D = S { t: { let i = 0; move || -> i32 { i } } };
| ^
|
note: however, the inferred type `S<[closure@$DIR/unnamable-types.rs:23:31: 23:45]>` cannot be named
note: however, the inferred type `S<[closure@unnamable-types.rs:23:31]>` cannot be named
--> $DIR/unnamable-types.rs:23:11
|
LL | const D = S { t: { let i = 0; move || -> i32 { i } } };
Expand Down
8 changes: 8 additions & 0 deletions src/test/ui/typeck/typeck_type_placeholder_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,3 +220,11 @@ fn value() -> Option<&'static _> {

const _: Option<_> = map(value);
//~^ ERROR the placeholder `_` is not allowed within types on item signatures for constants

fn evens_squared(n: usize) -> _ {
//~^ ERROR the placeholder `_` is not allowed within types on item signatures for return types
(1..n).filter(|x| x % 2 == 0).map(|x| x * x)
}

const _: _ = (1..10).filter(|x| x % 2 == 0).map(|x| x * x);
//~^ ERROR the placeholder `_` is not allowed within types on item signatures for constants
23 changes: 22 additions & 1 deletion src/test/ui/typeck/typeck_type_placeholder_item.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,27 @@ LL | const _: Option<_> = map(value);
| not allowed in type signatures
| help: replace with the correct type: `Option<u8>`

error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types
--> $DIR/typeck_type_placeholder_item.rs:224:31
|
LL | fn evens_squared(n: usize) -> _ {
| ^
| |
| not allowed in type signatures
| help: replace with an appropriate return type: `impl Iterator<Item = usize>`

error[E0121]: the placeholder `_` is not allowed within types on item signatures for constants
--> $DIR/typeck_type_placeholder_item.rs:229:10
|
LL | const _: _ = (1..10).filter(|x| x % 2 == 0).map(|x| x * x);
| ^ not allowed in type signatures
|
note: however, the inferred type `Map<Filter<Range<i32>, [closure@typeck_type_placeholder_item.rs:229:29]>, [closure@typeck_type_placeholder_item.rs:229:49]>` cannot be named
--> $DIR/typeck_type_placeholder_item.rs:229:14
|
LL | const _: _ = (1..10).filter(|x| x % 2 == 0).map(|x| x * x);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0121]: the placeholder `_` is not allowed within types on item signatures for functions
--> $DIR/typeck_type_placeholder_item.rs:140:31
|
Expand Down Expand Up @@ -636,7 +657,7 @@ LL | const D: _ = 42;
| not allowed in type signatures
| help: replace with the correct type: `i32`

error: aborting due to 69 previous errors
error: aborting due to 71 previous errors

Some errors have detailed explanations: E0121, E0282, E0403.
For more information about an error, try `rustc --explain E0121`.