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

#[derive] Debug, PartialEq, Hash, etc. for any function pointers, regardless of type signature #54508

Closed
fschutt opened this issue Sep 23, 2018 · 10 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@fschutt
Copy link
Contributor

fschutt commented Sep 23, 2018

Suppose you have a situation like this:

trait MyTrait { }

// This works:
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
struct WorkingFunctionPointer<T: MyTrait>(fn(T));

// This doesn't work:
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
struct NotWorkingFunctionPointer<T: MyTrait>(fn(&T));

https://play.rust-lang.org/?gist=d1bd43980abfb37197a8aaf84ed7b529&version=stable&mode=debug&edition=2015

You can implement this manually by writing something like this:

// #[derive(Debug, Clone, PartialEq, Hash, Eq)] for NotWorkingFunctionPointer<T>

impl<T: Layout> fmt::Debug for NotWorkingFunctionPointer<T> {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "NotWorkingFunctionPointer(0x{:x})", self.0 as usize)
    }
}

impl<T: Layout> Clone for NotWorkingFunctionPointer<T> {
    fn clone(&self) -> Self {
        NotWorkingFunctionPointer(self.0.clone())
    }
}

impl<T: Layout> Hash for NotWorkingFunctionPointer<T> {
  fn hash<H>(&self, state: &mut H) where H: Hasher {
    state.write_usize(self.0 as usize);
  }
}

impl<T: Layout> PartialEq for NotWorkingFunctionPointer<T> {
  fn eq(&self, rhs: &Self) -> bool {
    self.0 as usize == rhs.0 as usize
  }
}

impl<T: Layout> Eq for NotWorkingFunctionPointer<T> { }
impl<T: Layout> Copy for NotWorkingFunctionPointer<T> { }

... but this is tedious to do and leads to a lot of boilerplaite code. Even worse, this is especially bad if you have a FunctionPointer<T> used in a struct like this:

// #[derive] won't work here, same error!
struct Something<T: MyTrait> {
    ptr: NotWorkingFunctionPointer<T>,
    // other fields for demonstration
    blah: Blah,
    foo: Foo,
    baz: Baz,
}

... because then #[derive] doesn't work on the Something struct! This means you have to copy-paste all over again:

// A manual #[derive(Debug, Clone, PartialEq, Hash, Eq)] for Something<T>

impl<T: Layout> fmt::Debug for Something<T> {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "Something { ptr: {:?}, blah: {:?}, foo: {:?}, baz: {:?} }", self.ptr, self.blah, self.foo, self.baz)
    }
}

impl<T: MyTrait> Clone for Something<T> {
    fn clone(&self) -> Self {
         Something {
             ptr: self.ptr.clone(),
             blah: self.blah.clone(),
             foo: self.foo.clone(),
             baz: self.baz.clone(),
         }
    }
}

impl<T: MyTrait> Clone for Something<T> {
    fn clone(&self) -> Self {
         Something {
             ptr: self.ptr.clone(),
             blah: self.blah.clone(),
             foo: self.foo.clone(),
             baz: self.baz.clone(),
         }
    }
}

impl<T: Layout> Hash for Something<T> {
  fn hash<H>(&self, state: &mut H) where H: Hasher {
    state.hash(self.ptr);
    state.hash(self.foo);
    state.hash(self.bar);
    state.hash(self.baz);
  }
}

impl<T: Layout> PartialEq for Something<T> {
  fn eq(&self, rhs: &Self) -> bool {
    self.ptr == rhs.ptr &&
    self.foo == rhs.foo &&
    self.bar == rhs.bar &&
    self.baz == rhs.baz &&
  }
}

impl<T: Layout> Eq for Something<T> { }
impl<T: Layout> Copy for Something<T> { }

.. and over and over and over again, for each struct that you wrap / use Something in. Nevermind that this is error-prone if you add a field to the Something struct, don't forget to update the hash() and fmt() functions! This leads to a whole bunch of code that I need to copy-paste because derive doesn't work.

The real-world code where I encountered this problem is:

https://github.com/maps4print/azul/blob/4f2ba2e6eebdd0718d1adb15aac34c643f0f94ca/src/dom.rs#L159-L228
https://github.com/maps4print/azul/blob/4f2ba2e6eebdd0718d1adb15aac34c643f0f94ca/src/dom.rs#L358-L394
https://github.com/maps4print/azul/blob/4f2ba2e6eebdd0718d1adb15aac34c643f0f94ca/src/dom.rs#L413-L452

It's just stupid, copy-pasted code and if possible, I'd like to get rid of it with derive, but right now I sadly can't. My manual code is just a workaround for now, and I'd like this to be properly fixed somehow. Thanks in advance for any help.

@csmoe
Copy link
Member

csmoe commented Sep 23, 2018

what's your local rustc meta info?
the first snippet compiles actually https://play.rust-lang.org/?gist=2f8baac6a2adbf13ebf815ea1c839e13&version=stable&mode=debug&edition=2015

@fschutt
Copy link
Contributor Author

fschutt commented Sep 23, 2018

This is certainly weird - It seems to be fixed and even if I try older compiler versions, it doesn't appear. But I did get errors that Clone is not implemented for FunctionPointer<T> when using derive (even though Clone was implemented, like in the example). I know that this didn't work roughly a month ago.

I'm closing this for now, since it doesn't seem to be an issue, but I'll reopen this once I get a verifiable example of the problem working.

@fschutt fschutt closed this as completed Sep 23, 2018
@fschutt
Copy link
Contributor Author

fschutt commented Sep 23, 2018

Yeah, the example was picked poorly. The issue only appears when the callback carries a &T or &mut T not when it carries a T iself.

Working: https://play.rust-lang.org/?gist=e7a2e2890208509158e701f7497db199&version=stable&mode=debug&edition=2015

Not working: https://play.rust-lang.org/?gist=e805684b57c9992a9b1da66510c8c336&version=stable&mode=debug&edition=2015

All I changed was this:

#[derive(Debug, Clone, PartialEq, Hash, Eq)]
pub struct Callback<T: Layout>(pub fn(WindowInfo<T>, usize, usize) -> Dom<T>);

to this:

#[derive(Debug, Clone, PartialEq, Hash, Eq)]
pub struct Callback<T: Layout>(pub fn(&mut WindowInfo<T>, usize, usize) -> Dom<T>);

Notice the &mut, it's the only difference (but my code needs the callback to modify the data it's given, that's why I noticed it in the real-world code). This really seems like it's an unintended bug. I'll update the main example, so people don't get confused.

@fschutt fschutt reopened this Sep 23, 2018
@fschutt
Copy link
Contributor Author

fschutt commented Sep 23, 2018

It seems that rust thinks that &mut WindowInfo<T> refers to a trait object (?), but it doesn't. It's just a generic function pointer that takes a &mut Something<T> as the first argument, it's not a trait object of any kind.

I'm still not sure where the actual bug is coming from.

@csmoe
Copy link
Member

csmoe commented Sep 24, 2018

The key point is PartialEq/Eq isn't implemented for HRTB.

You introduced a HRTB in the broken one with &mut WindowInfo<T>, so the full type of the function(as the error message shows) is

for<'r> fn(&'r mut WindowInfo<T>, usize, usize) -> Dom<T>

cc http://rust-lang.github.io/rfcs/0387-higher-ranked-trait-bounds.html#update-syntax-of-fn-types
and for the working snippet, the full type of fn is

fn(WindowInfo<T>, usize, usize) -> Dom<T>

cc https://doc.rust-lang.org/std/cmp/trait.Eq.html#implementors
you can see that Eq is implemented for impl<Ret, A, B, C> Eq for fn(A, B, C) -> Ret, but not for HRTB.

@Havvy Havvy added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Sep 25, 2018
@fschutt
Copy link
Contributor Author

fschutt commented Sep 26, 2018

So my question then is... why isn't it implemented? Or is this the actual bug? Even after rust-lang/rfcs#387 is merged (four years ago?), this still doesn't work, so maybe it's just an issue in the standard library? But you can't impl derive for all possible function pointers:

impl<Ret, A, B, C> Eq for fn(A, B, C) -> Ret

impl<Ret, A, B, C> Eq for fn<'a>(&'a A, B, C) -> Ret
impl<Ret, A, B, C> Eq for fn<'a>(&mut 'a A, B, C) -> Ret

impl<Ret, A, B, C> Eq for fn<'a, 'b>(&'a A, &'b B, C) -> Ret
impl<Ret, A, B, C> Eq for fn<'a, 'b>(&mut 'a A, &'b B, C) -> Ret
impl<Ret, A, B, C> Eq for fn<'a, 'b>(&mut 'a A, &mut 'b B, C) -> Ret
impl<Ret, A, B, C> Eq for fn<'a, 'b>(&'a A, &mut 'b B, C) -> Ret
impl<Ret, A, B, C> Eq for fn<'a, 'b>(A, &mut 'b B, C) -> Ret
impl<Ret, A, B, C> Eq for fn<'a, 'b>(A, & 'b B, C) -> Ret

impl<Ret, A, B, C> Eq for fn<'a, 'b, 'c>(&'a A, &'b B, &'c C) -> Ret
impl<Ret, A, B, C> Eq for fn<'a, 'b, 'c>(&mut 'a A, &'b B, &'c C) -> Ret
impl<Ret, A, B, C> Eq for fn<'a, 'b, 'c>(&mut 'a A, &mut 'b B, &mut 'c C) -> Ret
impl<Ret, A, B, C> Eq for fn<'a, 'b, 'c>(&mut 'a A, &'b B, &mut 'c C) -> Ret
impl<Ret, A, B, C> Eq for fn<'a, 'b, 'c>(&'a A, &'b B, &mut 'c C) -> Ret

// ... and so on.

This is insane. You can't prepare for any function signature. What I don't get is that it's a function pointer - regardless of the type system, a function pointer is just a number, like a usize, so it should be comparable and derivable, regardless of the function signature. Essentially:

impl PartialEq for fn<*>(*) -> * {
     fn eq(&self: other: &Self) -> bool { self as usize == other as usize }
}

... where the * stands for "anything", i.e. any number or arguments or lifetime of types - fn<*>(*) -> * would simply be a type that represents "all function pointers". Why does the derivability of a function pointer depend on the function signature, that's what I don't really get. The lifetimes and such are accounted for when the function pointer is used, not when it's stored or created. Why does this need such a crazy complex type system hack, when a function pointer is just a number?

So maybe, I thought, this should be a compiler built-in thing, that the compiler knows about function pointers and just treats them as a usize. Or, you'd need some syntax for "any number of T, where for each T, the following traits are implemented" (my * syntax), i.e.:

// instead of:
impl<A> Eq for (A) where    A: Eq + ?Sized
impl<A, B> Eq for (A, B) where    A: Eq,    B: Eq + ?Sized
impl<A, B, C> Eq for (A, B, C) where    A: Eq,    B: Eq,    C: Eq + ?Sized
impl<A, B, C, D> Eq for (A, B, C, D) where    A: Eq,    B: Eq,    C: Eq,    D: Eq + ?Sized
impl<A, B, C, D, E> Eq for (A, B, C, D, E) where    A: Eq,    B: Eq,    C: Eq,    D: Eq,    E: Eq + ?Sized
impl<A, B, C, D, E, F> Eq for (A, B, C, D, E, F) where    A: Eq,    B: Eq,    C: Eq,    D: Eq,    E: Eq,    F: Eq + ?Sized
impl<A, B, C, D, E, F, G> Eq for (A, B, C, D, E, F, G) where    A: Eq,    B: Eq,    C: Eq,    D: Eq,    E: Eq,    F: Eq,    G: Eq + ?Sized
impl<A, B, C, D, E, F, G, H> Eq for (A, B, C, D, E, F, G, H) where    A: Eq,    B: Eq,    C: Eq,    D: Eq,    E: Eq,    F: Eq,    G: Eq,    H: Eq + ?Sized
impl<A, B, C, D, E, F, G, H, I> Eq for (A, B, C, D, E, F, G, H, I) where    A: Eq,    B: Eq,    C: Eq,    D: Eq,    E: Eq,    F: Eq,    G: Eq,    H: Eq,    I: Eq + ?Sized
impl<A, B, C, D, E, F, G, H, I, J> Eq for (A, B, C, D, E, F, G, H, I, J) where    A: Eq,    B: Eq,    C: Eq,    D: Eq,    E: Eq,    F: Eq,    G: Eq,    H: Eq,    I: Eq,    J: Eq + ?Sized
impl<A, B, C, D, E, F, G, H, I, J, K> Eq for (A, B, C, D, E, F, G, H, I, J, K) where    A: Eq,    B: Eq,    C: Eq,    D: Eq,    E: Eq,    F: Eq,    G: Eq,    H: Eq,    I: Eq,    J: Eq,    K: Eq + ?Sized

// just:
impl<*> Eq for (*) where *: Eq + ?Sized

@fschutt fschutt changed the title #[derive] should integrate with manual derived implementations on generic structs #[derive] Debug, PartialEq, Hash, etc. for any function pointers, regardless of type signature Sep 26, 2018
@jesskfullwood
Copy link

Pretty sure this is related to/the same as #46989

@bkolligs
Copy link

bkolligs commented Mar 18, 2023

Has anything like this been merged? I'd like to put an arbitrary function pointer inside of a std::collections::HashSet for a callback system. Something like this:

use std::collections::HashSet;

fn callback1(arg1: &str, arg2: &Vec<String>) {}

fn main(){
    let set: HashSet<fn(&str, &Vec<String>)> = HashSet::new();
    HashSet::insert(callback1);
    set.remove(callback1);
}

@XrXr
Copy link
Contributor

XrXr commented Mar 22, 2023

Has anything like this been merged?

No, but I think this will be fixed by #108080. See #108080 (comment)

@Dylan-DPC
Copy link
Member

Closing this as it is fixed now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants