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

Impl self methods not checking impl self constraints #5046

Closed
esdrubal opened this issue Aug 28, 2023 · 1 comment · Fixed by #6583
Closed

Impl self methods not checking impl self constraints #5046

esdrubal opened this issue Aug 28, 2023 · 1 comment · Fixed by #6583
Assignees
@esdrubal
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

Comments

@esdrubal
Copy link
Contributor

esdrubal commented Aug 28, 2023 •
edited
Loading

As found by @bitzoic in #5045

This compiles:

struct MyStruct {
    val: u64
}

abi MyContract {
    #[storage(read, write)]
    fn test_function();
}

storage {
    my_map: StorageMap<MyStruct, u64> = StorageMap {},
}

impl MyContract for Contract {
    #[storage(read, write)]
    fn test_function() {
        let my_struct = MyStruct { val: 1 };
        storage.my_map.insert(my_struct, 2)
    }
}

When this does not, even though the insert() function uses the same sha256<T>() function:

use std::hash::sha256;

struct MyStruct {
    val: u64
}

abi MyContract {
    fn test_function();
}

impl MyContract for Contract {
    fn test_function() {
        let my_struct = MyStruct { val: 1 };
        let result = sha256(my_struct);
    }
}

Originally posted by @bitzoic in #5045 (comment)

Seems like storage.my_map.insert(my_struct, 2)should not compile because my_struct does not implement the Hash trait, the insert should inherit the constraints from the impl self but somehow it does not detect the missing constraint.
@esdrubal esdrubal mentioned this issue Aug 28, 2023
Merged
7 tasks
@esdrubal esdrubal self-assigned this Aug 28, 2023
@esdrubal esdrubal added 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 bug Something isn't working labels Aug 28, 2023
@esdrubal
Copy link
Contributor Author

Contrary to what is reported the following code does not compile and gives the expected error:

struct MyStruct {
    val: u64
}

abi MyContract {
    #[storage(read, write)]
    fn test_function();
}

storage {
    my_map: StorageMap<MyStruct, u64> = StorageMap::<MyStruct, u64> {},
}

impl MyContract for Contract {
    #[storage(read, write)]
    fn test_function() {
        let my_struct = MyStruct { val: 1 };
        storage.my_map.insert(my_struct, 2)
    }
}
11 | 
12 | storage {
13 |     my_map: StorageMap<MyStruct, u64> = StorageMap::<MyStruct, u64> {},
   |                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Trait "Hash" is not implemented for type "MyStruct".
14 | }

The following code is also throwing the expected error:

trait Hash {
    fn hash(self) -> u64;
}

struct Struct<T> {
}

struct Struct2<T> {
}

struct StructHashable {
}

/*
impl Hash for StructHashable {
    fn hash(self) -> u64 {
        42
    }
}
*/

impl<T> Struct<Struct2<T>> where T: Hash {
    fn call_hash(self, hashable: T) -> u64 {
        hashable.hash()
    }
}

fn main() {
    let a = Struct::<Struct2<StructHashable>> {};
    let b = StructHashable {};
    a.call_hash(b);
}
32 |     let b = StructHashable {};
33 |     a.call_hash(b);
   |       ^^^^^^^^^ Trait "Hash" is not implemented for type "StructHashable".
34 | }

esdrubal added a commit that referenced this issue Sep 23, 2024
@esdrubal
Proof that #5046 is fixed.
2dea5dd 
The reproduction reported in #5046 is throwing the expected error.

Closes #5046.
@esdrubal esdrubal mentioned this issue Sep 23, 2024
Merged
8 tasks
esdrubal added a commit that referenced this issue Sep 26, 2024
@esdrubal @JoshuaBatty
Proof that #5046 is fixed. (#6583)
6fe4223 
## Description

The reproduction reported in #5046 is throwing the expected error.

Closes #5046.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] 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.
- [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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@esdrubal esdrubal

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
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Proof that #5046 is fixed. FuelLabs/sway
1 participant
@esdrubal