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

Trait name resolution does not work properly in the presence of trait type arguments #5036

Closed
anton-trunov opened this issue Aug 25, 2023 · 0 comments · Fixed by #5141
Closed
Assignees
Labels
bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen

Comments

@anton-trunov
Copy link
Contributor

anton-trunov commented Aug 25, 2023

Problem statement

Consider the following code snippet, which is basically the Eq trait parameterized with a dummy type parameter T (very similar to the example in issue #5018):

script;

trait Eq<T> {
    fn eq(self, other: Self) -> bool;
} {
    fn neq(self, other: Self) -> bool {
        __eq((self.eq(other)), false)
    }
}

impl Eq<u64> for u64 {
    fn eq(self, other: Self) -> bool {
        __eq(self, other)
    }
}

fn main() -> u64 {
    // block const evaluation for `x` (it does not currently support asm-blocks)
    let x = asm(x: 42u64) { x: u64 };
    let y = 1u64;
    if x.neq(y) {
        2
    } else {
        101
    }
}

This script fails to compile:

error
  --> /Users/anton/fuel/sway/test/src/e2e_vm_tests/test_programs/should_pass/language/name_resolution_after_monomorphization/name_resolution_inside_intrinsics/src/main.sw:4:8
  |
2 |
3 | trait Eq<T> {
4 |     fn eq(self, other: Self) -> bool;
  |        ^^ Internal compiler error: Method eq_1 is a trait method dummy and was not properly replaced.
Please file an issue on the repository and include the code that triggered this error.
5 | } {
6 |     fn neq(self, other: Self) -> bool {
  |
____

  Aborting due to 1 error.

Why this happens

This happens because the IR generator resolves eq into a dummy function with the empty body (just like in issue #4770).
In this particular case it happens because monomorphization of the Eq trait declaration during the typechecking of impl Eq<u64> for u64 creates the new version of the Eq trait with new decl refs, but the body of the neq impl method does not get updated with new references to the trait's interface surface, i.e. the monomorphized neq method keeps referring to the non-monomorphized eq method and name resolution fails.

@anton-trunov anton-trunov added bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen labels Aug 25, 2023
@anton-trunov anton-trunov self-assigned this Aug 25, 2023
IGI-111 pushed a commit that referenced this issue Sep 26, 2023
…pl` (#5141)

Closes #5036

## Description

Please see the linked issue

## 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).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] 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.
anton-trunov added a commit that referenced this issue Oct 3, 2023
## Description

Fixes #4877

## Tasks

- [x] Make a simple example with a trait and an impl for it compile
- [x] Fix typechecking for empty enums
- [x] Fix typechecking for trait impl methods
- [x] Fix typechecking for impl self blocks
- [x] Fix the case of non-implemented supertraits
- [x] Fix typechecking of generic traits
- [x] Fix typechecking of generic impl self blocks
- [x] Resolve ambiguity between term/path-level `Self` and the
type-level `Self` type parameter (related PR #4459):
       - [x] enums
       - [x] structs (deferred as issue #5164)
- [x] Fix `Self` for type constraints
- [x] Fix looping during insertion of trait constraints for generic
tuples (inserted `occurs_check` to prevent unification of a generic `A`
with a tuple `(A, A)`;
- [x] Fix exponential growth of the traits map (around 30 tests out of
more than 680 are failing now); related PR: #5004
- [x] Fix `Self` type for blanket implementations
- [x] Fix `Self` for associated consts
- [x] Fix name resolution issues for some tests with traits (like
`should_pass/language/eq_intrinsic`); blocking issues:
       - #5018 
       - #5036
- [x] Fix `should_fail/generic_traits` test
- [x] Fix `should_pass/language/where_clause_impls` test
- [x] Fix `should_pass/language/name_resolution_inside_intrinsics` test
- [x] Remove some commented out code in `impl_trait.rs`
- [x] Comment new code
- [x] **Disable** `should_pass/language/associated_type_container` test
- [x] **Disable** `should_pass/language/associated_type_method` test
- [x] **Disable**
`should_pass/language/associated_type_and_associated_const` test
- [x] **Disable** `should_pass/language/associated_type_iterator` test

## 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).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] 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.

---------

Co-authored-by: Joshua Batty <[email protected]>
Co-authored-by: IGI-111 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant