Point error span at Some constructor argument when trait resolution fails

[Nathan-Fenner]

This is a follow up to #108254 and #106477 which extends error span refinement to handle a case which I mistakenly believed was handled in #106477. The goal is to refine the error span depicted below:

trait Fancy {}
impl <T> Fancy for Option<T> where T: Iterator {}

fn want_fancy<F>(f: F) where F: Fancy {}

fn example() {
    want_fancy(Some(5));
//  (BEFORE)   ^^^^^^^ `{integer}` is not an iterator 
//  (AFTER)         ^  `{integer}` is not an iterator
}

I had used a (slightly more complex) example as an illustrative example in #108254 , but hadn't actually turned it into a test, because I had (incorrectly) believed at the time it was covered by existing behavior. It turns out that Some is slightly "special" in that it resolves differently from the other enum constructors I had tried, and therefore this test was actually broken.

I've now updated the tests to include this example, and fixed the code to correctly resolve the Some constructor so that the span of the error is reduced.

[rustbot]

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

[Nathan-Fenner]

r? @WaffleLapkin you spotted this issue in the previous PR

[WaffleLapkin]

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 */

[Nathan-Fenner]

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.

[WaffleLapkin]

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!

[WaffleLapkin]

Thanks!
@bors r+ rollup

[bors]

📌 Commit be15f17 has been approved by WaffleLapkin

It is now in the queue for this repository.