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

Support Duck-typing in filter predicate #608

Open
Acaccia opened this issue Jan 28, 2025 · 3 comments
Open

Support Duck-typing in filter predicate #608

Acaccia opened this issue Jan 28, 2025 · 3 comments
Assignees
Labels
bug Something isn't working Size: 🐮 L (2-4 weeks)

Comments

@Acaccia
Copy link
Collaborator

Acaccia commented Jan 28, 2025

The interpreter allows this kind of expression:

(define-private (foo (a (response int bool))) (and (is-ok a) (< (unwrap-panic a) 100)))
(define-private (bar (a (response int uint))) (and (is-ok a) (> (unwrap-panic a) 42))) 

(filter bar (filter foo (list (ok 1) (ok 50))))

Here we filter two times on a list which is inferred to have the type (list 2 (response int NoType)).
However, in the interpreter, the type of the list is inferred after the first call to filter to (list 2 (response int bool)), so the second call to filter fails as it expects another type for the list element.

We need to find a way to support this.

UPDATE: "The Workaround" also has a fix for fold, so it might need the same kind of adaptation for it too.

@Acaccia Acaccia added bug Something isn't working Size: 🐮 L (2-4 weeks) labels Jan 28, 2025
@obycode
Copy link
Collaborator

obycode commented Jan 28, 2025

This is the kind of thing that I would be fine with diverging from the interpreter for. We planned to roll out clarity-wasm with a hard fork any way in case there are unexpected differences. The downside is that if any of those differences show up on chain, then we would not be able to use clarity-wasm to speed up booting from genesis, using the clarity-wasm before that hard-fork.

@Acaccia
Copy link
Collaborator Author

Acaccia commented Jan 28, 2025

@obycode We cannot ignore this issue. Currently, some valid contracts won't run on Clarity-wasm because of the typechecking of filter. We had to come with this workaround to make things work, but it's not even foolproof yet (see #492).
To make all contracts work, we can either fix this issue, or we can rewrite a second typechecker from scratch. This issue is way easier I believe.

@obycode
Copy link
Collaborator

obycode commented Jan 28, 2025

You're definitely more familiar with it than I am currently, so I defer to whatever you think. I just thought that previous problems with the type checker were at least logically consistent. In this case, it seems like it would be reasonable to mark the expression as type (response int bool) and report a type error when it tries to cast it to a (response int uint).

@Acaccia Acaccia self-assigned this Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Size: 🐮 L (2-4 weeks)
Projects
None yet
Development

No branches or pull requests

2 participants