gtk-rs / glib

DEPRECATED, use https://github.com/gtk-rs/gtk-rs-core repository instead!
http://gtk-rs.org/
MIT License
93 stars 62 forks source link

Remove PhantomData from struct created in glib_object_wrapper! #713

Closed ids1024 closed 4 years ago

ids1024 commented 4 years ago

I don't believe these any reason to use PhantomData in a struct like this that isn't generic.

glib itself, at least, still compiles and passes tests with this simply removed.

ids1024 commented 4 years ago

Am I missing something, or would deriving PartialEq, Eq, PartialOrd, and Ord have the desired behavior, since it will pass them on to the wrapped ObjectRef type? It would be cleaner if they don't have to be explicitly implemented.

Similarly, it looks like the main reason the explict Debug is needed is so it won't display the PhantomData member.

sdroege commented 4 years ago

Similarly, it looks like the main reason the explict Debug is needed is so it won't display the PhantomData member.

That's also needed to additional print the type name :)

ids1024 commented 4 years ago

That's also needed to additional print the type name :)

Current Debug implementation:

SimpleObject { inner: ObjectRef { inner: 0x55bf0f179000, type: SimpleObject } }

Derived implementation:

SimpleObject(ObjectRef { inner: 0x56013db81000, type: SimpleObject })

So it looks like deriving debug is fine here unless printing it as a tuple struct is undesirable. (And if the first is preferable, the struct should probably just be defined as a struct with a inner member).

ids1024 commented 4 years ago

Since it appears to be correct, I've pushed a commit deriving PartialOrd, PartialEq, Eq, and Debug, as I suggested earlier.

sdroege commented 4 years ago

The relevant documentation is btw https://doc.rust-lang.org/nomicon/phantom-data.html