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

For types which implement Deref, change some methods to associated functions? #210

Closed
Tracked by #671
joshlf opened this issue Aug 1, 2023 · 0 comments · Fixed by #1618
Closed
Tracked by #671

For types which implement Deref, change some methods to associated functions? #210

joshlf opened this issue Aug 1, 2023 · 0 comments · Fixed by #1618
Labels
blocking-next-release This issue should be resolved before we release on crates.io compatibility-breaking Changes that are (likely to be) breaking

Comments

@joshlf
Copy link
Member

joshlf commented Aug 1, 2023

We have some types (such as Ref and Unalign) which implement Deref, and which provide methods (such as into_ref). This conflicts with the standard library's pattern of preferring associated functions over methods on types which implement Deref (e.g., Box::into_raw). This pattern is well-motivated: When a method is called on a Deref type, it is resolved on both the type itself and the target type (e.g., on both Box<T> and on T), and so methods on the type itself (Box<T>) can conflict with those on the target type (T). We should consider adopting this pattern.

@joshlf joshlf added the blocking-next-release This issue should be resolved before we release on crates.io label Aug 1, 2023
@joshlf joshlf added compatibility-breaking Changes that are (likely to be) breaking and removed blocking-next-release This issue should be resolved before we release on crates.io labels Aug 12, 2023
@joshlf joshlf mentioned this issue Aug 20, 2023
@joshlf joshlf mentioned this issue Dec 4, 2023
87 tasks
@joshlf joshlf added the blocking-next-release This issue should be resolved before we release on crates.io label May 30, 2024
joshlf added a commit that referenced this issue Sep 7, 2024
Since `Ref` implements `Deref`, methods risk conflicting with methods of
the same names on the target type.

Note that, in #210, we considered applying the same change to `Unalign`.
We choose not to do that because most uses of `Unalign` involve types
with alignments greater than 1, and for these types, `Unalign` does not
implement `Deref`. It's not worth making the API significantly more
cumbersome in order to make it easier for this niche use case.

Closes #210
joshlf added a commit that referenced this issue Sep 7, 2024
Since `Ref` implements `Deref`, methods risk conflicting with methods of
the same names on the target type.

Note that, in #210, we considered applying the same change to `Unalign`.
We choose not to do that because most uses of `Unalign` involve types
with alignments greater than 1, and for these types, `Unalign` does not
implement `Deref`. It's not worth making the API significantly more
cumbersome in order to make it easier for this niche use case.

Closes #210
github-merge-queue bot pushed a commit that referenced this issue Sep 8, 2024
Since `Ref` implements `Deref`, methods risk conflicting with methods of
the same names on the target type.

Note that, in #210, we considered applying the same change to `Unalign`.
We choose not to do that because most uses of `Unalign` involve types
with alignments greater than 1, and for these types, `Unalign` does not
implement `Deref`. It's not worth making the API significantly more
cumbersome in order to make it easier for this niche use case.

Closes #210
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking-next-release This issue should be resolved before we release on crates.io compatibility-breaking Changes that are (likely to be) breaking
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant