Skip to content

Bootstrap derive(PartialEq) (#163)#473

Merged
tyranron merged 23 commits into
masterfrom
163-eq-derive
May 29, 2025
Merged

Bootstrap derive(PartialEq) (#163)#473
tyranron merged 23 commits into
masterfrom
163-eq-derive

Conversation

@tyranron

@tyranron tyranron commented May 26, 2025

Copy link
Copy Markdown
Collaborator

Part of #163

Synopsis

This PR is the initial part of #163 and adds only structural expansion of derive(PartialEq).

Solution

Mirrors the PartialEq expansion of std, but applies trait bounds for generics correctly.

Checklist

  • Documentation is updated
  • Tests are added/updated
  • CHANGELOG entry is added

@tyranron tyranron added this to the 2.1.0 milestone May 26, 2025
@tyranron tyranron self-assigned this May 26, 2025
@tyranron tyranron added enhancement feature New derive or feature labels May 26, 2025
@tyranron

tyranron commented May 26, 2025

Copy link
Copy Markdown
Collaborator Author

I plan to add more features in the follow-up PRs:

  • Implement PartialEq::ne() method (std omits it) to allow better performance for cases when it's optimized.
  • Support #[partial_eq(skip)] attribute.
  • Support #[partial_eq(where(<bounds>))] attribute (required when fields may contain private types).
  • Implement forwarding expansion for newtypes (single-field structs) with #[partial_eq(forward)] support.
  • Finally, support #[partial_eq(<types>)] to allow the case requested in Feature idea: PartialEq and PartialOrd for the wrapped type #163.
  • Implement derive(Eq) expanding as impl <T> Eq for Type<T> where Self: PartialEq {}.

tyranron added a commit that referenced this pull request May 27, 2025
Revealed from #473

## Synopsis

`utils::GenericsSearch` doesn't consider associative type, leading to
missing trait bounds in `AsRef`/`AsMut` expansions.

```rust
#[derive(AsRef)]
#[as_ref(forward)]
struct Forward<T: Some> {
    first: T::Assoc,
}
```
expands as:
```rust
#[allow(deprecated)]
#[allow(unreachable_code)]
#[automatically_derived]
impl<T: Some, __AsT: ?derive_more::core::marker::Sized> derive_more::core::convert::AsRef<__AsT> for Forward<T>
{
    #[inline]
    fn as_ref(&self) -> &__AsT { <T::Assoc as derive_more::core::convert::AsRef<__AsT>>::as_ref(&self.first) }
}
```
but should expand as:
```rust
#[allow(deprecated)]
#[allow(unreachable_code)]
#[automatically_derived]
impl<T: Some, __AsT: ?derive_more::core::marker::Sized> derive_more::core::convert::AsRef<__AsT> for Forward<T>
where
    T::Assoc: derive_more::core::convert::AsRef<__AsT>,
{
    #[inline]
    fn as_ref(&self) -> &__AsT { <T::Assoc as derive_more::core::convert::AsRef<__AsT>>::as_ref(&self.first) }
}
```

## Solution

This PR fixes this situation and covers associative types usage in
`AsRef`/`AsMut` tests.

## Additionally

Improves `utils::GenericsSearch` performance by early returns (stop
`Visit`ing whenever something is found).

Also, unit tests for `utils::GenericsSearch` are added.
@tyranron tyranron marked this pull request as ready for review May 27, 2025 21:24
@tyranron tyranron enabled auto-merge (squash) May 27, 2025 21:25
@tyranron tyranron requested a review from JelteF May 27, 2025 21:30
@tyranron

Copy link
Copy Markdown
Collaborator Author

@JelteF note: I named Cargo feature eq, because:

  1. I think it makes sense to keep both Eq and PartialEq derives under the same feature, as we do for AsRef/AsMut, and so, keep even the same documentation page for both of them.
  2. I think that using #[eq] helper attribute for derive(PartialEq) (like #[eq(skip)]) is more ergonomic, since derive(Eq) won't need any helper attributes at all.

@tyranron tyranron mentioned this pull request May 28, 2025
3 tasks
@tyranron

Copy link
Copy Markdown
Collaborator Author

ping @JelteF

@tyranron tyranron merged commit 47d8c10 into master May 29, 2025
17 checks passed
@tyranron tyranron deleted the 163-eq-derive branch May 29, 2025 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement feature New derive or feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature idea: PartialEq and PartialOrd for the wrapped type

2 participants