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

Point error span at Some constructor argument when trait resolution fails #108557

Merged
merged 3 commits into from
Mar 3, 2023
Merged

Point error span at Some constructor argument when trait resolution fails #108557

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2566b41
Point error span at Some constructor argument when trait resolution f…
Nathan-Fenner Feb 28, 2023
f0212e6
Fix error spans for arguments to tuple enum constructors
Nathan-Fenner Feb 28, 2023
be15f17
restore unaddressed FIXME
Nathan-Fenner Feb 28, 2023
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
14 changes: 10 additions & 4 deletions compiler/rustc_hir_typeck/src/fn_ctxt/adjust_fulfillment_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -714,12 +714,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.tcx.parent(expr_ctor_def_id)
}
hir::def::DefKind::Ctor(hir::def::CtorOf::Variant, hir::def::CtorKind::Fn) => {
// If this is a variant, its parent is the type definition.
if in_ty_adt.did() != self.tcx.parent(expr_ctor_def_id) {
// FIXME: Deal with type aliases?
if in_ty_adt.did() == self.tcx.parent(expr_ctor_def_id) {
// The constructor definition refers to the variant:
// For example, for a local type `MyEnum::MyVariant` triggers this case.
expr_ctor_def_id
} else if in_ty_adt.did() == self.tcx.parent(self.tcx.parent(expr_ctor_def_id))
{
// The constructor definition refers to the "constructor" of the variant:
// For example, `Some(5)` triggers this case.
Comment on lines +730 to +731
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand... Does Some's def id refer to the use statement?

// (3) and then parent's parent refers to the option itself
enum Option<T> {
    Some(T), // (2) The first parent refers here
    None,
}

pub use Option::Some; // (1) `Some`'s def id refers to here

If this is the case, would this "fail" on something like the following?:

enum A<T> { V0(T) }
use A::V0 as V1;
use V1 as V2;
/* then, code with V2, such that you need parent's parent's parent */

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's not pointing to the use statement. It's pointing to a "constructor" definition which lives inside the "variant" definition.

So if we have

enum ExampleTuple<T> {
    ExampleTupleVariant(T),
}
use ExampleDifferentTupleVariantName as ExampleYetAnotherTupleVariantName;
use ExampleTuple as ExampleOtherTuple;
use ExampleTuple::ExampleTupleVariant as ExampleDifferentTupleVariantName;
use ExampleTuple::*;

impl<A> T1 for ExampleTuple<A> where A: T3 {}

then for all of

  • ExampleTuple::ExampleTupleVariant(q)
  • ExampleTupleVariant(q)
  • ExampleOtherTuple::ExampleTupleVariant(q)
  • ExampleDifferentTupleVariantName(q)
  • ExampleYetAnotherTupleVariantName(q)

by printing the relevant DefIds we see the same thing in each case:

expr_ctor_def_id :::                                   DefId(0:29 ~ blame_trait_error[b442]::ExampleTuple::ExampleTupleVariant::{constructor#0})
self.tcx.parent(expr_ctor_def_id) :::                  DefId(0:28 ~ blame_trait_error[b442]::ExampleTuple::ExampleTupleVariant)
self.tcx.parent(self.tcx.parent(expr_ctor_def_id)) ::: DefId(0:26 ~ blame_trait_error[b442]::ExampleTuple)

So going up once gets us to the variant and going up twice gets us to the type definition. I'm actually not sure how/why going up only once ever seemed to work before - this may have just been a miss since the first PR focused mostly on structs and tuples.

I've added a lot of new tests to cover this name resolving behavior.

Copy link
Member

Choose a reason for hiding this comment

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

Ohhh, and for structures you get expr_ctor_def_id ::: DefId(...::Struct::{constructor#0}), so you only need to go one level up the parent chain. Makes sense, thanks for the explanation and adding a comment!

self.tcx.parent(expr_ctor_def_id)
} else {
return Err(expr);
Nathan-Fenner marked this conversation as resolved.
Show resolved Hide resolved
}
expr_ctor_def_id
}
_ => {
return Err(expr);
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/errors/trait-bound-error-spans/blame-trait-error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,24 @@ struct Burrito<F> {
filling: F,
}

impl<It: Iterator> T1 for Option<It> {}

impl<'a, A: T1> T1 for &'a A {}

fn want<V: T1>(_x: V) {}

fn example<Q>(q: Q) {
want(Wrapper { value: Burrito { filling: q } });
//~^ ERROR the trait bound `Q: T3` is not satisfied [E0277]

want(Some(()));
//~^ ERROR `()` is not an iterator [E0277]

want(Some(q));
//~^ ERROR `Q` is not an iterator [E0277]

want(&Some(q));
//~^ ERROR `Q` is not an iterator [E0277]
}

fn main() {}
81 changes: 78 additions & 3 deletions tests/ui/errors/trait-bound-error-spans/blame-trait-error.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0277]: the trait bound `Q: T3` is not satisfied
--> $DIR/blame-trait-error.rs:24:46
--> $DIR/blame-trait-error.rs:28:46
|
LL | want(Wrapper { value: Burrito { filling: q } });
| ---- ^ the trait `T3` is not implemented for `Q`
Expand All @@ -21,7 +21,7 @@ LL | impl<B: T2> T1 for Wrapper<B> {}
| |
| unsatisfied trait bound introduced here
note: required by a bound in `want`
--> $DIR/blame-trait-error.rs:21:12
--> $DIR/blame-trait-error.rs:25:12
|
LL | fn want<V: T1>(_x: V) {}
| ^^ required by this bound in `want`
Expand All @@ -30,6 +30,81 @@ help: consider restricting type parameter `Q`
LL | fn example<Q: T3>(q: Q) {
| ++++

error: aborting due to previous error
error[E0277]: `()` is not an iterator
--> $DIR/blame-trait-error.rs:31:15
|
LL | want(Some(()));
| ---- ^^ `()` is not an iterator
| |
| required by a bound introduced by this call
|
= help: the trait `Iterator` is not implemented for `()`
= help: the trait `T1` is implemented for `Option<It>`
note: required for `Option<()>` to implement `T1`
--> $DIR/blame-trait-error.rs:21:20
|
LL | impl<It: Iterator> T1 for Option<It> {}
| -------- ^^ ^^^^^^^^^^
| |
| unsatisfied trait bound introduced here
note: required by a bound in `want`
--> $DIR/blame-trait-error.rs:25:12
|
LL | fn want<V: T1>(_x: V) {}
| ^^ required by this bound in `want`

error[E0277]: `Q` is not an iterator
--> $DIR/blame-trait-error.rs:34:15
|
LL | want(Some(q));
| ---- ^ `Q` is not an iterator
| |
| required by a bound introduced by this call
|
note: required for `Option<Q>` to implement `T1`
--> $DIR/blame-trait-error.rs:21:20
|
LL | impl<It: Iterator> T1 for Option<It> {}
| -------- ^^ ^^^^^^^^^^
| |
| unsatisfied trait bound introduced here
note: required by a bound in `want`
--> $DIR/blame-trait-error.rs:25:12
|
LL | fn want<V: T1>(_x: V) {}
| ^^ required by this bound in `want`
help: consider restricting type parameter `Q`
|
LL | fn example<Q: std::iter::Iterator>(q: Q) {
| +++++++++++++++++++++

error[E0277]: `Q` is not an iterator
--> $DIR/blame-trait-error.rs:37:16
|
LL | want(&Some(q));
| ---- ^ `Q` is not an iterator
| |
| required by a bound introduced by this call
|
note: required for `Option<Q>` to implement `T1`
--> $DIR/blame-trait-error.rs:21:20
|
LL | impl<It: Iterator> T1 for Option<It> {}
| -------- ^^ ^^^^^^^^^^
| |
| unsatisfied trait bound introduced here
= note: 1 redundant requirement hidden
= note: required for `&Option<Q>` to implement `T1`
note: required by a bound in `want`
--> $DIR/blame-trait-error.rs:25:12
|
LL | fn want<V: T1>(_x: V) {}
| ^^ required by this bound in `want`
help: consider restricting type parameter `Q`
|
LL | fn example<Q: std::iter::Iterator>(q: Q) {
| +++++++++++++++++++++

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0277`.
4 changes: 2 additions & 2 deletions tests/ui/errors/traits/blame-trait-error-spans-on-exprs.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ LL | fn example<Q: T3>(q: Q) {
| ++++

error[E0277]: the trait bound `Q: T3` is not satisfied
--> $DIR/blame-trait-error-spans-on-exprs.rs:93:27
--> $DIR/blame-trait-error-spans-on-exprs.rs:93:53
|
LL | want(Wrapper { value: TacoKinds::OneTaco(false, q) });
| ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `T3` is not implemented for `Q`
| ---- ^ the trait `T3` is not implemented for `Q`
| |
| required by a bound introduced by this call
|
Expand Down