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 all commits
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
20 changes: 17 additions & 3 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,26 @@ 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) {
// For a typical enum like
// `enum Blah<T> { Variant(T) }`
// we get the following resolutions:
// - expr_ctor_def_id ::: DefId(0:29 ~ source_file[b442]::Blah::Variant::{constructor#0})
// - self.tcx.parent(expr_ctor_def_id) ::: DefId(0:28 ~ source_file[b442]::Blah::Variant)
// - self.tcx.parent(self.tcx.parent(expr_ctor_def_id)) ::: DefId(0:26 ~ source_file[b442]::Blah)

// Therefore, we need to go up once to obtain the variant and up twice to obtain the type.
// Note that this pattern still holds even when we `use` a variant or `use` an enum type to rename it, or chain `use` expressions
// together; this resolution is handled automatically by `qpath_res`.

// FIXME: Deal with type aliases?
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 {
// FIXME: Deal with type aliases?
return Err(expr);
Nathan-Fenner marked this conversation as resolved.
Show resolved Hide resolved
}
expr_ctor_def_id
}
_ => {
return Err(expr);
Expand Down
74 changes: 74 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,85 @@ 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) {}

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 {}

enum ExampleStruct<T> {
ExampleStructVariant { field: T },
}
use ExampleDifferentStructVariantName as ExampleYetAnotherStructVariantName;
use ExampleStruct as ExampleOtherStruct;
use ExampleStruct::ExampleStructVariant as ExampleDifferentStructVariantName;
use ExampleStruct::*;

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

struct ExampleActuallyTupleStruct<T>(T, i32);
use ExampleActuallyTupleStruct as ExampleActuallyTupleStructOther;

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

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]

want(&ExampleTuple::ExampleTupleVariant(q));
//~^ ERROR `Q: T3` is not satisfied [E0277]

want(&ExampleTupleVariant(q));
//~^ ERROR `Q: T3` is not satisfied [E0277]

want(&ExampleOtherTuple::ExampleTupleVariant(q));
//~^ ERROR `Q: T3` is not satisfied [E0277]

want(&ExampleDifferentTupleVariantName(q));
//~^ ERROR `Q: T3` is not satisfied [E0277]

want(&ExampleYetAnotherTupleVariantName(q));
//~^ ERROR `Q: T3` is not satisfied [E0277]

want(&ExampleStruct::ExampleStructVariant { field: q });
//~^ ERROR `Q: T3` is not satisfied [E0277]

want(&ExampleStructVariant { field: q });
//~^ ERROR `Q: T3` is not satisfied [E0277]

want(&ExampleOtherStruct::ExampleStructVariant { field: q });
//~^ ERROR `Q: T3` is not satisfied [E0277]

want(&ExampleDifferentStructVariantName { field: q });
//~^ ERROR `Q: T3` is not satisfied [E0277]

want(&ExampleYetAnotherStructVariantName { field: q });
//~^ ERROR `Q: T3` is not satisfied [E0277]

want(&ExampleActuallyTupleStruct(q, 0));
//~^ ERROR `Q: T3` is not satisfied [E0277]

want(&ExampleActuallyTupleStructOther(q, 0));
//~^ ERROR `Q: T3` is not satisfied [E0277]
}

fn main() {}
Loading