rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.28k stars 1.61k forks source link

wrong go to function #11759

Open mimoo opened 2 years ago

mimoo commented 2 years ago

rust-analyzer version: 5fae65dd2 2022-03-07 stable

rustc version: rustc 1.58.0 (02072b482 2022-01-11)

relevant settings: none

Overview

First, some easy to follow screenshots to convince you: this eval is a closure:

Screen Shot 2022-03-18 at 11 46 33 AM

But the function we're passing it to is not expecting a closure:

Screen Shot 2022-03-18 at 11 47 05 AM

that's the function it should have pointed me to. Interestingly sourcegraph has the same issues, so I guess they must be using rust-analyzer under the hood.

Screen Shot 2022-03-18 at 11 49 19 AM

I reckon this bug comes from the fact that we abuse a bit generic parameters:

pub type JointLookupSpec<F> = JointLookup<SingleLookup<F>>;

Repro steps

Sorry in advance, I haven't been able to do a smaller repro steps:

  1. clone this repo
  2. git checkout fa87bd7cdb5adf7df43c187a2828d2e588088fb2
  3. check kimchi/src/circuits/polynomials/lookup.rs:656
mrmr1993 commented 2 years ago

This is the relevant code; presumably it's due to the re-use of the name evaluate.

impl<F: Zero + One + Clone> JointLookup<F> {
    // TODO: Support multiple tables
    /// Evaluate the combined value of a joint-lookup.
    pub fn evaluate(&self, joint_combiner: F) -> F {
        combine_table_entry(joint_combiner, self.entry.iter())
    }
}

impl<F: Copy> JointLookup<SingleLookup<F>> {
    pub fn reduce<K, G: Fn(LocalPosition) -> K>(&self, eval: &G) -> JointLookup<K>
    where
        K: Zero,
        K: Mul<F, Output = K>,
    {
        JointLookup {
            table_id: self.table_id,
            entry: self.entry.iter().map(|s| s.evaluate(eval)).collect(),
        }
    }

    pub fn evaluate<K, G: Fn(LocalPosition) -> K>(&self, joint_combiner: K, eval: &G) -> K
    where
        K: Zero + One + Clone,
        K: Mul<F, Output = K>,
    {
        self.reduce(eval).evaluate(joint_combiner)
    }
}
flodiebold commented 2 years ago

Pretty sure this has the same root cause as #5441.

lowr commented 2 years ago

So this is the minimal reproducible example I could get (playground):

struct Inner<F>(F);

struct Outer<S>(S);

trait C {}

impl<F: C> Outer<F> {
    fn foo(&self, not_ok: F) {} // candidate 1
}

impl<F> Outer<Inner<F>> {
    fn foo(&self, ok: usize, eval: ()) {} // candidate 2
}

trait Trait: C {
    type O: AnotherTrait;
}

trait AnotherTrait: Trait {}

fn should_compile<F: Trait>(spec: Outer<Inner<F>>) {
    spec.foo(2, ());
}

Apparently we should select candidate 2 for spec.foo(2, ()) but we choose candidate 1. Chalk somehow thinks Inner<F> implements C:

caching solution Ok(Ambig(Unknown)) for UCanonical {
    canonical: Canonical {
        value: InEnvironment {
            environment: Env([for<> FromEnv(!0_0: Trait), for<> FromEnv(!0_0: Sized)]),
            goal: FromEnv(Inner<[?0 := !0_0]>: C),
        },
        binders: [],
    },
    universes: 1,
}

I'm not a Chalk expert, but it kinda looks like https://github.com/rust-lang/chalk/issues/727? FWIW this confusion seems to originate in Field trait from ark-ff crate, which has recusive structure like this:

trait Field {
    type BasePrimeField: PrimeField;
}
pub trait PrimeField: Field<BasePrimeField = Self> {}