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.

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

Deref coercions don't work on blocks when the target is sized #57749

Open
lambda-fairy opened this issue Jan 19, 2019 · 6 comments
Open

Deref coercions don't work on blocks when the target is sized #57749

lambda-fairy opened this issue Jan 19, 2019 · 6 comments
Labels
A-coercions Area: implicit and explicit `expr as Type` coercions C-bug Category: This is a bug. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@lambda-fairy
Copy link
Contributor

As seen on StackOverflow.

Consider the following code:

use std::ops::Deref;

fn main() {
    fn save(who: &str) {
        println!("I'll save you, {}!", who);
    }
    
    struct Madoka;
    
    impl Deref for Madoka {
        type Target = str;
        fn deref(&self) -> &Self::Target {
            "Madoka"
        }
    }
    
    save(&{ Madoka });

    fn reset(how: &u32) {
        println!("Reset {} times", how);
    }

    struct Homura;

    impl Deref for Homura {
        type Target = u32;
        fn deref(&self) -> &Self::Target {
            &42
        }
    }

    // Uncomment this to get a type error
    // reset(&{ Homura });
}

The following expressions do compile:

  • reset(&Homura)
  • reset(&&{ Homura })
  • reset({ &Homura })

But this doesn't:

  • reset(&{ Homura })
error[E0308]: mismatched types
  --> src/main.rs:17:14
   |
17 |     reset(&{ Homura });
   |              ^^^^^^ expected u32, found struct `main::Homura`
   |
   = note: expected type `u32`
              found type `main::Homura`

Is there are particular reason for this discrepancy?

I found two similar issues #26978 and #32122 but they don't seem to cover this particular case.

@lambda-fairy
Copy link
Contributor Author

Background: I found this issue while playing with wasm-bindgen and web-sys. The code I was trying to write looked like this:

let body = document.body().unwrap();
body.append_child(&{
    let button = document.create_element("button")?.dyn_into::<HtmlButtonElement>().unwrap();
    button.append_child(&document.create_text_node("Click me"))?;
    button
})?;

append_child takes a Node, and HtmlButtonElement implements a chain of Derefs that ends in Node.

@lambda-fairy
Copy link
Contributor Author

From reading https://github.com/rust-lang/rust/pull/31651/files I get the impression that str and [T] and dyn Trait are special-cased somehow, but I don't have the background knowledge to pursue that further.

@estebank estebank added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jan 19, 2019
@ldm0
Copy link
Contributor

ldm0 commented May 4, 2020

Linked issue: #23014

@QuineDot
Copy link

Another use case came up on URLO: the desugared ? operator:

// `bar: fn(&B)`, `a: Result<A, E>`, `A: Sized + Deref<Target=B>`
// This fails but `&*a?` or `&&a?` or `a?.deref()` work
bar(&a?);
More code context
use std::ops::Deref;

struct A;
struct B;

impl Deref for A {
    type Target = B;
    fn deref(&self) -> &B {
        &B
    }
}

// Replace the above with these and it will work:
//type A = String;
//type B = str;

struct E;

fn foo(a: Result<A, E>) -> Result<(), E> {
    // &*a? or &&a? or a?.deref() also work
    bar(&a?);
    Ok(())
}

fn bar(_: &B) {}

Playground

@AaronKutch
Copy link
Contributor

I found a workaround that works on stable for this:

struct InnerBase {/* actual data here */}
struct Base([InnerBase; 1]);
#[repr(transparent)] // for the transmute
struct Ref([InnerBase]);

impl std::ops::Deref for Base {
    type Target = Ref;

    fn deref<'a>(&'a self) -> &'a Ref {
        // safety: the lifetimes and types are spelled out, and `&Ref` should be identical to `&[InnerBase]`
        unsafe { std::mem::transmute::<&[InnerBase], &Ref>(&*std::ptr::slice_from_raw_parts(self.0.as_ptr(), 1)) }
    }
}

impl std::ops::Deref for Ref {
    type Target = InnerBase;

    fn deref<'a>(&'a self) -> &'a InnerBase {
        &self.0[0]
    }
}

impl InnerBase {
    // will work as if it were implemented for `&Ref`
    pub fn hello_world(&self) {}
}

// no error
let r: &Ref = &{ Base([InnerBase {}; 1]) };
r.hello_world();

not sure if there is a safer way to do this, trying to make Ref unsized some other way rapidly multiplies the unsafety

@AaronKutch
Copy link
Contributor

Update: It turns out that my above solution could technically turn into UB because it wasn't repr(C) and the compiler would technically be able to reorder the fat pointer internals between types, even if Zrandomize-layout wouldn't trigger it (https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Can.20we.20stabilize.20the.20layout.20of.20Vec.3CT.3E.20and.20.26.5BT.5D.3F), here is a more rigorous and simpler solution

#[repr(C)] // needed for `internal_as_ref*`, also this needs to be identical to `RefType` minus the fake DST
pub struct Base {
    pub _actual_data: ...,
}

#[repr(C)]
pub struct RefType {
    _boo: PhantomData<... if needed ...>,
    _actual_data: ...,
    _dst: [()],
}

impl<'a> Base {
    pub(in crate::mimick) fn internal_as_ref(&'a self) -> &'a RefType {
        // important: use a Fat pointer cast instead of transmute
        let bits = ptr::slice_from_raw_parts(self as *const Self, 0) as *const RefType;
        unsafe { &*bits }
    }

    pub(in crate::mimick) fn internal_as_mut(&'a mut self) -> &'a mut RefType {
        let bits = ptr::slice_from_raw_parts_mut(self as *mut Self, 0) as *mut RefType;
        unsafe { &mut *bits }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-coercions Area: implicit and explicit `expr as Type` coercions C-bug Category: This is a bug. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants