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.

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

Use root obligation on E0277 for some cases #121826

Merged
merged 3 commits into from
Mar 5, 2024
Merged

Use root obligation on E0277 for some cases #121826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f0c9311
Use root obligation on E0277 for some cases
estebank Feb 29, 2024
89a3c19
Be more lax in `.into_iter()` suggestion when encountering `Iterator`…
estebank Mar 1, 2024
40f9dcc
Use `can_eq` instead of `Ty<'_> == Ty<'_>`
estebank Mar 2, 2024
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
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return false;
}

match ty.kind() {
match ty.peel_refs().kind() {
ty::Param(param) => {
let generics = self.tcx.generics_of(self.body_id);
let generic_param = generics.type_param(&param, self.tcx);
Expand All @@ -184,7 +184,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
}
ty::Alias(ty::AliasKind::Opaque, _) => {
ty::Slice(..) | ty::Adt(..) | ty::Alias(ty::AliasKind::Opaque, _) => {
for unsatisfied in unsatisfied_predicates.iter() {
if is_iterator_predicate(unsatisfied.0, self.tcx) {
return true;
Expand Down
59 changes: 54 additions & 5 deletions compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,9 +416,59 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
ty::PredicateKind::Clause(ty::ClauseKind::Trait(trait_predicate)) => {
let trait_predicate = bound_predicate.rebind(trait_predicate);
let trait_predicate = self.resolve_vars_if_possible(trait_predicate);
let trait_ref = trait_predicate.to_poly_trait_ref();

if let Some(guar) = self.emit_specialized_closure_kind_error(&obligation, trait_ref) {
// Let's use the root obligation as the main message, when we care about the
// most general case ("X doesn't implement Pattern<'_>") over the case that
// happened to fail ("char doesn't implement Fn(&mut char)").
//
// We rely on a few heuristics to identify cases where this root
// obligation is more important than the leaf obligation:
let (main_trait_predicate, o) = if let ty::PredicateKind::Clause(
ty::ClauseKind::Trait(root_pred)
) = root_obligation.predicate.kind().skip_binder()
&& !trait_predicate.self_ty().skip_binder().has_escaping_bound_vars()
&& !root_pred.self_ty().has_escaping_bound_vars()
// The type of the leaf predicate is (roughly) the same as the type
// from the root predicate, as a proxy for "we care about the root"
// FIXME: this doesn't account for trivial derefs, but works as a first
// approximation.
&& (
// `T: Trait` && `&&T: OtherTrait`, we want `OtherTrait`
self.can_eq(
obligation.param_env,
trait_predicate.self_ty().skip_binder(),
root_pred.self_ty().peel_refs(),
)
// `&str: Iterator` && `&str: IntoIterator`, we want `IntoIterator`
|| self.can_eq(
obligation.param_env,
trait_predicate.self_ty().skip_binder(),
root_pred.self_ty(),
)
)
// The leaf trait and the root trait are different, so as to avoid
// talking about `&mut T: Trait` and instead remain talking about
// `T: Trait` instead
&& trait_predicate.def_id() != root_pred.def_id()
// The root trait is not `Unsize`, as to avoid talking about it in
// `tests/ui/coercion/coerce-issue-49593-box-never.rs`.
&& Some(root_pred.def_id()) != self.tcx.lang_items().unsize_trait()
{
(
self.resolve_vars_if_possible(
root_obligation.predicate.kind().rebind(root_pred),
),
root_obligation,
)
} else {
(trait_predicate, &obligation)
};
let trait_ref = main_trait_predicate.to_poly_trait_ref();

if let Some(guar) = self.emit_specialized_closure_kind_error(
&obligation,
trait_ref,
) {
return guar;
}

Expand Down Expand Up @@ -459,8 +509,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
notes,
parent_label,
append_const_msg,
} = self.on_unimplemented_note(trait_ref, &obligation, &mut long_ty_file);

} = self.on_unimplemented_note(trait_ref, o, &mut long_ty_file);
let have_alt_message = message.is_some() || label.is_some();
let is_try_conversion = self.is_try_conversion(span, trait_ref.def_id());
let is_unsize =
Expand All @@ -483,7 +532,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
};

let err_msg = self.get_standard_error_message(
&trait_predicate,
&main_trait_predicate,
message,
predicate_is_const,
append_const_msg,
Expand Down
3 changes: 1 addition & 2 deletions library/core/src/future/future.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ use crate::task::{Context, Poll};
#[lang = "future_trait"]
#[diagnostic::on_unimplemented(
label = "`{Self}` is not a future",
message = "`{Self}` is not a future",
note = "{Self} must be a future or must implement `IntoFuture` to be awaited"
message = "`{Self}` is not a future"
)]
pub trait Future {
/// The type of value produced on completion.
Expand Down
5 changes: 5 additions & 0 deletions library/core/src/future/into_future.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ use crate::future::Future;
/// ```
#[stable(feature = "into_future", since = "1.64.0")]
#[rustc_diagnostic_item = "IntoFuture"]
#[diagnostic::on_unimplemented(
label = "`{Self}` is not a future",
message = "`{Self}` is not a future",
note = "{Self} must be a future or must implement `IntoFuture` to be awaited"
)]
pub trait IntoFuture {
/// The output that the future will produce on completion.
#[stable(feature = "into_future", since = "1.64.0")]
Expand Down
43 changes: 43 additions & 0 deletions library/core/src/iter/traits/collect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,49 @@ pub trait FromIterator<A>: Sized {
/// ```
#[rustc_diagnostic_item = "IntoIterator"]
#[rustc_skip_array_during_method_dispatch]
#[rustc_on_unimplemented(
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
on(
_Self = "core::ops::range::RangeTo<Idx>",
label = "if you meant to iterate until a value, add a starting value",
note = "`..end` is a `RangeTo`, which cannot be iterated on; you might have meant to have a \
bounded `Range`: `0..end`"
),
on(
_Self = "core::ops::range::RangeToInclusive<Idx>",
label = "if you meant to iterate until a value (including it), add a starting value",
note = "`..=end` is a `RangeToInclusive`, which cannot be iterated on; you might have meant \
to have a bounded `RangeInclusive`: `0..=end`"
),
on(
_Self = "[]",
label = "`{Self}` is not an iterator; try calling `.into_iter()` or `.iter()`"
),
on(_Self = "&[]", label = "`{Self}` is not an iterator; try calling `.iter()`"),
on(
_Self = "alloc::vec::Vec<T, A>",
label = "`{Self}` is not an iterator; try calling `.into_iter()` or `.iter()`"
),
on(
_Self = "&str",
label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`"
),
on(
_Self = "alloc::string::String",
label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`"
),
on(
_Self = "{integral}",
note = "if you want to iterate between `start` until a value `end`, use the exclusive range \
syntax `start..end` or the inclusive range syntax `start..=end`"
),
on(
_Self = "{float}",
note = "if you want to iterate between `start` until a value `end`, use the exclusive range \
syntax `start..end` or the inclusive range syntax `start..=end`"
),
label = "`{Self}` is not an iterator",
message = "`{Self}` is not an iterator"
)]
#[stable(feature = "rust1", since = "1.0.0")]
pub trait IntoIterator {
/// The type of the elements being iterated over.
Expand Down
35 changes: 2 additions & 33 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 @@ -28,42 +28,11 @@ fn _assert_is_object_safe(_: &dyn Iterator<Item = ()>) {}
#[rustc_on_unimplemented(
on(
_Self = "core::ops::range::RangeTo<Idx>",
label = "if you meant to iterate until a value, add a starting value",
note = "`..end` is a `RangeTo`, which cannot be iterated on; you might have meant to have a \
bounded `Range`: `0..end`"
note = "you might have meant to use a bounded `Range`"
),
on(
_Self = "core::ops::range::RangeToInclusive<Idx>",
label = "if you meant to iterate until a value (including it), add a starting value",
note = "`..=end` is a `RangeToInclusive`, which cannot be iterated on; you might have meant \
to have a bounded `RangeInclusive`: `0..=end`"
),
on(
_Self = "[]",
label = "`{Self}` is not an iterator; try calling `.into_iter()` or `.iter()`"
),
on(_Self = "&[]", label = "`{Self}` is not an iterator; try calling `.iter()`"),
on(
_Self = "alloc::vec::Vec<T, A>",
label = "`{Self}` is not an iterator; try calling `.into_iter()` or `.iter()`"
),
on(
_Self = "&str",
label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`"
),
on(
_Self = "alloc::string::String",
label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`"
),
on(
_Self = "{integral}",
note = "if you want to iterate between `start` until a value `end`, use the exclusive range \
syntax `start..end` or the inclusive range syntax `start..=end`"
),
on(
_Self = "{float}",
note = "if you want to iterate between `start` until a value `end`, use the exclusive range \
syntax `start..end` or the inclusive range syntax `start..=end`"
note = "you might have meant to use a bounded `RangeInclusive`"
),
label = "`{Self}` is not an iterator",
message = "`{Self}` is not an iterator"
Expand Down
5 changes: 2 additions & 3 deletions tests/ui/associated-types/substs-ppaux.normal.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,12 @@ help: use parentheses to call this function
LL | let x: () = foo::<'static>();
| ++

error[E0277]: the size for values of type `str` cannot be known at compilation time
error[E0277]: the trait bound `str: Foo<'_, '_, u8>` is not satisfied
--> $DIR/substs-ppaux.rs:49:6
|
LL | <str as Foo<u8>>::bar;
| ^^^ doesn't have a size known at compile-time
| ^^^ the trait `Sized` is not implemented for `str`, which is required by `str: Foo<'_, '_, u8>`
Copy link
Member

@fmease fmease Mar 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I haven't read the implementation or the most recent Zulip comments and just saw this stderr diff, so sorry if this has been discussed already.

Is this intentional? Shouldn't we still use the #[rustc_on_unimplemented]/#[diagnostic::on_unimplemented] message here despite the derived/root obligation changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is somewhat intentional. When you have T: Sized then you want T to have a compile time known size. When you have T: Foo, Foo: Sized, I think it makes sense to go back to "T does not implement Sized", it's a subtle difference of "what does the user care about".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, I do want to do a full refactor of this logic, too many ad-hoc hacks have piled beyond maintainability (in no small part due to me), but now that we know what the output we want to keep and change is, it will be easier to clean this up in a way that makes sense. This PR is about putting the output piece closer to that "ideal end state" in anticipation of that work.

|
= help: the trait `Sized` is not implemented for `str`, which is required by `str: Foo<'_, '_, u8>`
note: required for `str` to implement `Foo<'_, '_, u8>`
--> $DIR/substs-ppaux.rs:11:17
|
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/associated-types/substs-ppaux.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,6 @@ fn foo<'z>() where &'z (): Sized {
//[normal]~| found fn item `fn() {foo::<'static>}`

<str as Foo<u8>>::bar;
//[verbose]~^ ERROR the size for values of type
//[normal]~^^ ERROR the size for values of type
//[verbose]~^ ERROR the trait bound `str: Foo<'?0, '?1, u8>` is not satisfied
//[normal]~^^ ERROR the trait bound `str: Foo<'_, '_, u8>` is not satisfied
}
5 changes: 2 additions & 3 deletions tests/ui/associated-types/substs-ppaux.verbose.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,12 @@ help: use parentheses to call this function
LL | let x: () = foo::<'static>();
| ++

error[E0277]: the size for values of type `str` cannot be known at compilation time
error[E0277]: the trait bound `str: Foo<'?0, '?1, u8>` is not satisfied
--> $DIR/substs-ppaux.rs:49:6
|
LL | <str as Foo<u8>>::bar;
| ^^^ doesn't have a size known at compile-time
| ^^^ the trait `Sized` is not implemented for `str`, which is required by `str: Foo<'?0, '?1, u8>`
|
= help: the trait `Sized` is not implemented for `str`, which is required by `str: Foo<'?0, '?1, u8>`
note: required for `str` to implement `Foo<'?0, '?1, u8>`
--> $DIR/substs-ppaux.rs:11:17
|
Expand Down
1 change: 0 additions & 1 deletion tests/ui/async-await/async-error-span.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ LL | fn get_future() -> impl Future<Output = ()> {
| ^^^^^^^^^^^^^^^^^^^^^^^^ `()` is not a future
|
= help: the trait `Future` is not implemented for `()`
= note: () must be a future or must implement `IntoFuture` to be awaited

error[E0282]: type annotations needed
--> $DIR/async-error-span.rs:13:9
Expand Down
2 changes: 0 additions & 2 deletions tests/ui/async-await/coroutine-not-future.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ LL | takes_future(returns_coroutine());
| required by a bound introduced by this call
|
= help: the trait `Future` is not implemented for `impl Coroutine<Yield = (), Return = ()>`
= note: impl Coroutine<Yield = (), Return = ()> must be a future or must implement `IntoFuture` to be awaited
note: required by a bound in `takes_future`
--> $DIR/coroutine-not-future.rs:17:26
|
Expand All @@ -69,7 +68,6 @@ LL | | });
| |_____^ `{coroutine@$DIR/coroutine-not-future.rs:41:18: 41:23}` is not a future
|
= help: the trait `Future` is not implemented for `{coroutine@$DIR/coroutine-not-future.rs:41:18: 41:23}`
= note: {coroutine@$DIR/coroutine-not-future.rs:41:18: 41:23} must be a future or must implement `IntoFuture` to be awaited
note: required by a bound in `takes_future`
--> $DIR/coroutine-not-future.rs:17:26
|
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/auto-traits/typeck-default-trait-impl-precedence.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ impl Signed for i32 { }
fn main() {
is_defaulted::<&'static i32>();
is_defaulted::<&'static u32>();
//~^ ERROR `u32: Signed` is not satisfied
//~^ ERROR the trait bound `&'static u32: Defaulted` is not satisfied
}
4 changes: 2 additions & 2 deletions tests/ui/auto-traits/typeck-default-trait-impl-precedence.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: the trait bound `u32: Signed` is not satisfied
error[E0277]: the trait bound `&'static u32: Defaulted` is not satisfied
--> $DIR/typeck-default-trait-impl-precedence.rs:19:20
|
LL | is_defaulted::<&'static u32>();
| ^^^^^^^^^^^^ the trait `Signed` is not implemented for `u32`, which is required by `&'static u32: Defaulted`
| ^^^^^^^^^^^^ the trait `Signed` is not implemented for `&'static u32`, which is required by `&'static u32: Defaulted`
|
note: required for `&'static u32` to implement `Defaulted`
--> $DIR/typeck-default-trait-impl-precedence.rs:10:19
Expand Down
1 change: 0 additions & 1 deletion tests/ui/coroutine/gen_block_is_no_future.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ LL | fn foo() -> impl std::future::Future {
| ^^^^^^^^^^^^^^^^^^^^^^^^ `{gen block@$DIR/gen_block_is_no_future.rs:5:5: 5:21}` is not a future
|
= help: the trait `Future` is not implemented for `{gen block@$DIR/gen_block_is_no_future.rs:5:5: 5:21}`
= note: {gen block@$DIR/gen_block_is_no_future.rs:5:5: 5:21} must be a future or must implement `IntoFuture` to be awaited

error: aborting due to 1 previous error

Expand Down
1 change: 0 additions & 1 deletion tests/ui/feature-gates/feature-gate-trivial_bounds.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ LL | fn use_for() where i32: Iterator {
| ^^^^^^^^^^^^^ `i32` is not an iterator
|
= help: the trait `Iterator` is not implemented for `i32`
= note: if you want to iterate between `start` until a value `end`, use the exclusive range syntax `start..end` or the inclusive range syntax `start..=end`
= help: see issue #48214
= help: add `#![feature(trivial_bounds)]` to the crate attributes to enable

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/for/issue-20605.current.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0277]: the size for values of type `dyn Iterator<Item = &'a mut u8>` cannot be known at compilation time
error[E0277]: `dyn Iterator<Item = &'a mut u8>` is not an iterator
--> $DIR/issue-20605.rs:5:17
|
LL | for item in *things { *item = 0 }
Expand Down
6 changes: 4 additions & 2 deletions tests/ui/for/issue-20605.next.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
error[E0277]: the trait bound `dyn Iterator<Item = &'a mut u8>: IntoIterator` is not satisfied
error[E0277]: `dyn Iterator<Item = &'a mut u8>` is not an iterator
--> $DIR/issue-20605.rs:5:17
|
LL | for item in *things { *item = 0 }
| ^^^^^^^ the trait `IntoIterator` is not implemented for `dyn Iterator<Item = &'a mut u8>`
| ^^^^^^^ `dyn Iterator<Item = &'a mut u8>` is not an iterator
|
= help: the trait `IntoIterator` is not implemented for `dyn Iterator<Item = &'a mut u8>`

error: the type `<dyn Iterator<Item = &'a mut u8> as IntoIterator>::IntoIter` is not well-formed
--> $DIR/issue-20605.rs:5:17
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/for/issue-20605.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

fn changer<'a>(mut things: Box<dyn Iterator<Item=&'a mut u8>>) {
for item in *things { *item = 0 }
//[current]~^ ERROR the size for values of type
//[next]~^^ ERROR the trait bound `dyn Iterator<Item = &'a mut u8>: IntoIterator` is not satisfied
//[current]~^ ERROR `dyn Iterator<Item = &'a mut u8>` is not an iterator
//[next]~^^ ERROR `dyn Iterator<Item = &'a mut u8>` is not an iterator
//[next]~| ERROR the type `<dyn Iterator<Item = &'a mut u8> as IntoIterator>::IntoIter` is not well-formed
//[next]~| ERROR the type `&mut <dyn Iterator<Item = &'a mut u8> as IntoIterator>::IntoIter` is not well-formed
//[next]~| ERROR the type `Option<<<dyn Iterator<Item = &'a mut u8> as IntoIterator>::IntoIter as Iterator>::Item>` is not well-formed
Expand Down
1 change: 0 additions & 1 deletion tests/ui/impl-trait/issues/issue-83919.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ LL | fn get_fut(&self) -> Self::Fut {
| ^^^^^^^^^ `{integer}` is not a future
|
= help: the trait `Future` is not implemented for `{integer}`
= note: {integer} must be a future or must implement `IntoFuture` to be awaited

error: aborting due to 1 previous error

Expand Down
1 change: 0 additions & 1 deletion tests/ui/issues-71798.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ LL | *x
| -- return type was inferred to be `u32` here
|
= help: the trait `Future` is not implemented for `u32`
= note: u32 must be a future or must implement `IntoFuture` to be awaited

error: aborting due to 2 previous errors

Expand Down
1 change: 0 additions & 1 deletion tests/ui/iterators/bound.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ LL | struct T(S<u8>);
| ^^^^^ `u8` is not an iterator
|
= help: the trait `Iterator` is not implemented for `u8`
= note: if you want to iterate between `start` until a value `end`, use the exclusive range syntax `start..end` or the inclusive range syntax `start..=end`
note: required by a bound in `S`
--> $DIR/bound.rs:1:13
|
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/iterators/vec-on-unimplemented.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
//@ run-rustfix
fn main() {
let _ = vec![true, false].into_iter().map(|v| !v).collect::<Vec<_>>();
//~^ ERROR no method named `map` found for struct `Vec<bool>` in the current scope
}
5 changes: 3 additions & 2 deletions tests/ui/iterators/vec-on-unimplemented.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//@ run-rustfix
fn main() {
vec![true, false].map(|v| !v).collect::<Vec<_>>();
//~^ ERROR `Vec<bool>` is not an iterator
let _ = vec![true, false].map(|v| !v).collect::<Vec<_>>();
//~^ ERROR no method named `map` found for struct `Vec<bool>` in the current scope
}
Loading
Loading