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

Partial equivalence #6883

Open
3 tasks
ironcev opened this issue Feb 3, 2025 · 0 comments
Open
3 tasks

Partial equivalence #6883

ironcev opened this issue Feb 3, 2025 · 0 comments
Assignees
Labels
lib: core Core library lib: std Standard library tracking-issue Tracking issue for experimental Sway features

Comments

@ironcev
Copy link
Member

ironcev commented Feb 3, 2025

This is a tracking issue for Partial equivalence.
The experimental feature flag for the issue is partial_eq.

Description

Currently, core::ops provides only the Eq trait, thus, allowing expressing equivalence, but not partial equivalence. In simple terms, we expect that for every type that supports comparison, each instance of the type is always equal to itself, a property called reflexivity. While this assumption holds for majority of types used in blockchain development, we want to lift this restriction and support partial equivalence. A typical example of a type supporting partial equivalence, but not equivalence, is a floating point number, where NaN is different from any other number, including itself: NaN != NaN.

The need for partial equivalence is first expressed in #6668.

Breaking Changes

Replace Eq trait implementations with PartialEq and Eq implementations

Partial equivalence feature renames the current Eq trait to PartialEq and adds a new, empty Eq trait with PartialEq as a supertrait.

Every existing Eq trait implementation needs to be renamed to PartialEq, and in addition, an empty Eq implementations needs to be added.

E.g., this implementation:

impl Eq for SomeType {
    fn eq(self, other: Self) -> bool {
        self.x == other.x
    }
}

will become:

impl PartialEq for SomeType {
    fn eq(self, other: Self) -> bool {
        self.x == other.x
    }
}

impl Eq for SomeType {}

If the original implementation implements Eq for a generic type and in addition has Eq on trait constraints, those Eq trait constraints must be replaced by PartialEq in the new PartialEq impl, and remain Eq in the new, empty, Eq impl.

E.g., this implementation:

impl<A, B> Eq for (A, B)
where
    A: Eq,
    B: Eq,
{
    fn eq(self, other: Self) -> bool {
        self.0 == other.0 && self.1 == other.1
    }
}

will become:

impl<A, B> PartialEq for (A, B)
where
    A: PartialEq,
    B: PartialEq,
{
    fn eq(self, other: Self) -> bool {
        self.0 == other.0 && self.1 == other.1
    }
}

impl<A, B> Eq for (A, B)
where
    A: Eq,
    B: Eq,
{}

After doing these replacements for Eq trait impls, consider looking at trait constraints (where keywords) in other impls that use Eq as a trait constraint, and eventually lowering the constraint by replacing it with the PartialEq. Note that this is not strictly necessary, because the above change did not change the existing semantics of Eq. It is just that some of the constraints might be lowered, if the full equivalence is not strictly needed.

If you decide to review the usages of Eq in wheres, the following regex-search will give you these usages: [^:]:[^:]\s*Eq.

Check name clashes with existing types called Error or Enum

The partial_eq feature introduces a new traits in the core prelude: core::ops::PartialEq.

If the existing code already has a type with that name, and only if that type is imported via glob (*) import the "Multiple bindings exist for symbol in the scope" error will be emitted.

E.g., suppose that we have a type my::impls::PartialEq, and import it in scope by use my::impls::*. After migrating to partial equivalence, the following error will appear:

error: Multiple bindings exist for symbol in the scope
   --> /src/main.sw:118:51
  |
1 | my_fn::<PartialEq>(14);
  |         ^^^^^ The following paths are all valid bindings for symbol "PartialEq": "core::ops::PartialEq" and "my::impls::PartialEq".
  |
  = help: Consider using a fully qualified name, e.g., `my::impls::PartialEq`.

The solution is, as proposed in the help message, to either fully qualify the PartialEq symbol, or to import it by specifying its full path: use my::impls::PartialEq.

Steps

@ironcev ironcev added lib: core Core library lib: std Standard library tracking-issue Tracking issue for experimental Sway features labels Feb 3, 2025
@ironcev ironcev self-assigned this Feb 3, 2025
ironcev added a commit that referenced this issue Feb 11, 2025
## Description

This PR:
- implements partial equivalence in the `core::ops`, as explained in
detail in #6883. The implementation is opt-in and behind the
`partial_eq` experimental feature flag.
- implements `forc migrate` migration steps for automatic migration to
`partial_eq` feature.
- extends the infrastructure for writing `forc migrate` migrations.
- preserves `Annotation`s on `LexedModule`s in the `LexedProgram`.
(Those were before stripped off and not available in the compiled
`Programs`. This resulted in loss off `//!` doc-comments that are
represented as `doc-comment` annotations.)

Extensions in the `forc migrate` infrastructure include:
- possibility to postpone migration steps. This allows writing
multi-step migrations where the first step is usually early adopting an
experimental feature by using `cfg` attributes in code. This possibility
is crucial for migrating our own codebase where we want to early-adopt
and test and stabilize experimental features.
- possibility to match elements on both mutable and immutable parsed
trees. The initial assumption that immutable matching on typed trees
will always be sufficient does not hold. Experimental `cfg` attributes
can remove parts of typed trees that might be needed in analysis.
- `LexedLocate(As)Annotated` for easier location and retrieval of
`Annotated` elements.
- additional implementations of existing traits.
- additional `Modifier`s for modifying existing elements in the lexed
tree.
- `New` struct for convenient creation of often needed lexed tree
elements. Note that we do not expect migrations to generate large new
parts of lexed trees, but mostly to modify or copy and modify existing
ones.

Almost all of the changes in the Sway files are done automatically by
the migration steps. The only manual change was in the `core` library
(`std` library is also automatically migrated) and in slight extension
of two of the tests.

Note that some of the tests have `Eq` traits in trait constraints. As
explained in the #6883, it is not necessary to change those to
`PartialEq`. We might consider changing some of them, for testing
purposes, and such a change is done on one of the tests that was testing
the behavior of the `Eq` trait. But for the majority of the tests, there
is no strict need for switching from `Eq` to `PartialEq`.

Rolling `PartialEq` out in the tests provoked three existing issues that
will be fixed in separate PRs: #6897, #6898, #6899.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib: core Core library lib: std Standard library tracking-issue Tracking issue for experimental Sway features
Projects
None yet
Development

No branches or pull requests

1 participant