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.

Sign up for GitHub

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

Jump to bottom

Fix compiler stack overflow when compiling code with PartialEq and Eq in trait constraints #6898

Open
Tracked by #6883
ironcev opened this issue Feb 6, 2025 · 0 comments
Open
Tracked by #6883

Fix compiler stack overflow when compiling code with PartialEq and Eq in trait constraints #6898

ironcev opened this issue Feb 6, 2025 · 0 comments
Labels
bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler General compiler. Should eventually become more specific as the issue is triaged team:compiler Compiler Team

Comments

@ironcev
Copy link
Member

ironcev commented Feb 6, 2025

This issue can be observed on some should_pass tests when they run with experimental feature partial_eq turned on.

When compiling those tests, compiler will overflow its stack.

The issue seams to appear on the intersection of trait constraints, supertraits, and resolution of equality operator's == corresponding eq methods.

Tests in which the issue appears contain a TODO with a link to this GitHub issue.
@ironcev ironcev added bug Something isn't working compiler General compiler. Should eventually become more specific as the issue is triaged compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen labels Feb 6, 2025
This was referenced Feb 6, 2025
Open
Merged
@IGI-111 IGI-111 added the team:compiler Compiler Team label Feb 10, 2025
ironcev added a commit that referenced this issue Feb 11, 2025
@ironcev
Implement partial equivalence and extend forc migrate tool (#6900)
bb7c99b 
## 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
Assignees
No one assigned
Labels
bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler General compiler. Should eventually become more specific as the issue is triaged team:compiler Compiler Team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ironcev @IGI-111