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

Should we really be implementing Deref and DerefMut #22

Closed
tcharding opened this issue Dec 29, 2024 · 5 comments
Closed

Should we really be implementing Deref and DerefMut #22

tcharding opened this issue Dec 29, 2024 · 5 comments

Comments

@tcharding
Copy link
Member

The Rust API guidlines state that only smart pointers should implement these two traits but we are doing so. Is this correct?

links:

@tcharding
Copy link
Member Author

If we implement Deref it implies Ordered is a smart pointer but then we are contradicting this rule:

https://rust-lang.github.io/api-guidelines/predictability.html#c-smart-ptr

@tcharding
Copy link
Member Author

After more thought I believe we are right to implement Deref even though Ordered is not a smart pointer. From the docs for Deref

When to implement Deref or DerefMut
The same advice applies to both deref traits. In general, deref traits should be implemented if:

  1. a value of the type transparently behaves like a value of the target type;
  2. the implementation of the deref function is cheap; and
  3. users of the type will not be surprised by any deref coercion behaviour.

ref: https://doc.rust-lang.org/std/ops/trait.Deref.html

To remove the contradition with C-SMART-PTR I did #25

@apoelstra
Copy link
Member

FWIW I agree, but if/when @Kixunil comes back I'll bet he complains about this.

@Kixunil
Copy link

Kixunil commented Jan 23, 2025

Aaand, now I'm going to complain! Just kidding! Actually, I think this is not abuse of Deref since it's not used to simulate OOP, it mostly obeys "a value of the type transparently behaves like a value of the target type" and there's a precedent for this in core - the ManuallyDrop type has Deref (though Drop affects program behavior much less than Ord).

@apoelstra
Copy link
Member

there's a precedent for this in core - the ManuallyDrop type

Ooh, interesting. And yes, the OO framing of "if the outer type can be substituted for the inner type with no changes then it's ok to Deref" sounds good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants