Skip to content

Regression: multiple deref implementations cause E0308 #17075

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.

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

Closed
mmastrac opened this issue Apr 15, 2024 · 2 comments · Fixed by #17529
Closed

Regression: multiple deref implementations cause E0308 #17075

mmastrac opened this issue Apr 15, 2024 · 2 comments · Fixed by #17529
Assignees
Labels
A-ty type system / type inference / traits / method resolution C-bug Category: bug

Comments

@mmastrac
Copy link

mmastrac commented Apr 15, 2024

Playground link (compiles): https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=70ffd01d215fbd49e02bc06970779e2b

Error in rust-analyzer:

expected &HandleScope<'static, u32>, found &CallbackScope<'static, {unknown}>"

This was working previously and seems to have broken recently. It appears that a compilation of:

  • Lifetime generics
  • Generics with defaults, and
  • Multiple Deref paths

contribute to a spurious E0308 error that appears in rust-analyzer but not in rust itself, nor any other tools that compile Rust.

This code was reduced from errors appearing in rusty_v8:

use std::{marker::PhantomData, ops::Deref};

#[derive(Default)]
pub struct HandleScope<'s, C = u32> { _p: Option<&'s PhantomData<C>> }

#[derive(Default)]
pub struct CallbackScope<'s, C = u32> {
    _p: Option<&'s PhantomData<C>>
}

impl <'s> CallbackScope<'s> {
    pub fn new() -> CallbackScope<'s> {
        Self::default()
    }
}

impl <'s> Deref for CallbackScope<'s> {
    type Target = HandleScope<'s>;
    fn deref(&self) -> &Self::Target {
        unsafe { std::mem::transmute(self) }
    }
}

impl <'s> Deref for CallbackScope<'s, ()> {
    type Target = HandleScope<'s, ()>;
    fn deref(&self) -> &Self::Target {
        unsafe { std::mem::transmute(self) }
    }
}

fn y(_: &HandleScope) {}

#[test]
pub fn test_deref() {
    let scope = &CallbackScope::new();
    y(scope);
    // "expected &HandleScope<'static, u32>, found &CallbackScope<'static, {unknown}>"
}
@mmastrac mmastrac added the C-bug Category: bug label Apr 15, 2024
@Veykril
Copy link
Member

Veykril commented Apr 15, 2024

Probably due to the lifetime var changes, we seem to struggle with generic param defaults now

@Veykril Veykril added the A-ty type system / type inference / traits / method resolution label Apr 15, 2024
@poliorcetics
Copy link
Contributor

Related to #17066

@Veykril Veykril self-assigned this Jun 28, 2024
bors added a commit that referenced this issue Jul 2, 2024
fix: Fix lifetime parameters moving parameter defaults

Fixes #17075, #17515

We were incorrectly filling the default params due to our odd order of lifetime parameters vs type/const params. So this needs some special handling so that we don't shift the defaults around.
bors added a commit that referenced this issue Jul 2, 2024
fix: Fix lifetime parameters moving parameter defaults

Fixes #17075, #17515

We were incorrectly filling the default params due to our odd order of lifetime parameters vs type/const params. So this needs some special handling so that we don't shift the defaults around.
@bors bors closed this as completed in 73c97e3 Jul 2, 2024
lnicola pushed a commit to lnicola/rust that referenced this issue Jul 11, 2024
fix: Fix lifetime parameters moving parameter defaults

Fixes rust-lang/rust-analyzer#17075, rust-lang/rust-analyzer#17515

We were incorrectly filling the default params due to our odd order of lifetime parameters vs type/const params. So this needs some special handling so that we don't shift the defaults around.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ty type system / type inference / traits / method resolution C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants