Rust-GCC / gccrs

GCC Front-End for Rust
https://rust-gcc.github.io/
GNU General Public License v2.0
2.39k stars 155 forks source link

`Deref` and `DerefMut` cause ambiguity issues when resolving #3032

Open CohenArthur opened 5 months ago

CohenArthur commented 5 months ago

This code is the base implementation of Deref and DerefMut:

#[lang = "sized"]
trait Sized {}

#[lang = "deref"]
pub trait Deref {
    /// The resulting type after dereferencing.
    #[stable(feature = "rust1", since = "1.0.0")]
    // #[rustc_diagnostic_item = "deref_target"]
    type Target: ?Sized;

    /// Dereferences the value.
    #[must_use]
    #[stable(feature = "rust1", since = "1.0.0")]
    // #[rustc_diagnostic_item = "deref_method"]
    fn deref(&self) -> &Self::Target;
}

impl<T: ?Sized> Deref for &T {
    type Target = T;

    fn deref(&self) -> &T {
        *self
    }
}

// this is added because of #3030 
extern "C" {
    fn never() -> !;
}

impl<T: ?Sized> !DerefMut for &T {
    fn deref_mut(&mut self) -> &mut T {
        unsafe { never() }
    }
}

impl<T: ?Sized> Deref for &mut T {
    type Target = T;

    fn deref(&self) -> &T {
        *self
    }
}

#[lang = "deref_mut"]
pub trait DerefMut: Deref {
    /// Mutably dereferences the value.
    #[stable(feature = "rust1", since = "1.0.0")]
    fn deref_mut(&mut self) -> &mut Self::Target;
}

impl<T: ?Sized> DerefMut for &mut T {
    fn deref_mut(&mut self) -> &mut T {
        *self
    }
}

If we remove the #[lang = "deref{_mut}"] attributes, then the code works, but otherwise it errors out:

arthur@platypus ~/G/r/gccrs (master)> build/gcc/crab1 test.rs
test.rs:5:5: error: ambiguous type bound for trait Deref and type &mut T
    5 | pub trait Deref {
      |     ^~~~~
......
   51 | impl<T: ?Sized> DerefMut for &mut T {
      |                              ~
test.rs:52:39: error: mismatched types, expected ‘&mut T’ but got ‘<placeholder:>’ [E0308]
   52 |     fn deref_mut(&mut self) -> &mut T {
      |     ~~                         ~      ^

the reason is that there is an ambiguity and that the compiler finds two associated trait impls: Deref for &mut T and Deref for &T

CohenArthur commented 5 months ago

the issue only arises upon adding this implementation:


impl<T: ?Sized> DerefMut for &mut T {
    fn deref_mut(&mut self) -> &mut T {
        *self
    }
}

which is part of the core library but isn't present in our existing testcases, as they focused on Deref and its behavior

CohenArthur commented 5 months ago

we're also running into this with this code example - I suspect the fix would be the same and there is something wrong when it comes to references/pointers and mutability:

#![feature(intrinsics)]

#[lang = "sized"]
trait Sized {}

pub struct VaListImpl<'f>;

mod sealed_trait {
    pub trait VaArgSafe {}
}

impl<T> sealed_trait::VaArgSafe for *mut T {}
impl<T> sealed_trait::VaArgSafe for *const T {}

impl<'f> VaListImpl<'f> {
    /// Advance to the next arg.
    #[inline]
    pub unsafe fn arg<T: sealed_trait::VaArgSafe>(&mut self) {
        va_arg2(self);
    }
}

fn va_arg2<T: sealed_trait::VaArgSafe>(ap: &mut VaListImpl<'_>) {}
CohenArthur commented 5 months ago

I don't even understand how it makes sense for the last example - the type bound is unused, so it shouldn't affect anything

philberty commented 14 hours ago

we're also running into this with this code example - I suspect the fix would be the same and there is something wrong when it comes to references/pointers and mutability:

#![feature(intrinsics)]

#[lang = "sized"]
trait Sized {}

pub struct VaListImpl<'f>;

mod sealed_trait {
    pub trait VaArgSafe {}
}

impl<T> sealed_trait::VaArgSafe for *mut T {}
impl<T> sealed_trait::VaArgSafe for *const T {}

impl<'f> VaListImpl<'f> {
    /// Advance to the next arg.
    #[inline]
    pub unsafe fn arg<T: sealed_trait::VaArgSafe>(&mut self) {
        va_arg2(self);
    }
}

fn va_arg2<T: sealed_trait::VaArgSafe>(ap: &mut VaListImpl<'_>) {}

This is a seperate issue i am going to open this into a seperate issue as the original issue is different and fix is up now