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

Check if typechecker_workaround works for nested filter/fold expressions #492

Open
matteojug opened this issue Aug 23, 2024 · 3 comments
Open

Comments

@matteojug
Copy link
Contributor

matteojug commented Aug 23, 2024

We may need to fix typechecker_workaround as currently it only operates on top level expressions, thus ignoring nested ones: (example from map issue, solved by using local workaround instead of typechecker_workaround)

#[test]
fn crosscheck_map_ok_buff(buf in buffer(50)) {
    let snippet = format!(r#"
    (define-private (foo (a (response (buff 50) int))) (len (unwrap! a u0)))
    (begin (map foo (list (ok {buf})))) ;; added a wrapping `begin` here to an otherwise passing test
    "#);

    crosscheck(
        &snippet,
        Ok(Some(Value::cons_list_unsanitized(vec![Value::UInt(50)]).unwrap()))
    )
}
Err(Wasm(UnableToLoadModule(WebAssembly translation error

Caused by:
    Invalid input WebAssembly code at offset 7800: type mismatch: expected i64, found i32

This may be impacting also filter/fold.

Originally posted by @matteojug in #474 (comment)

@Acaccia
Copy link
Collaborator

Acaccia commented Aug 23, 2024

It does impact filter and fold. I created in no time this failing example #466 (review)

I had this idea to have a Set containing the expressions fixed in typechecker_workaround, and to check if an expression is not in this set when we use WasmGenerator::set_expr_type (poorly explained here). However, I'm not sure this idea would work, or it might need some reworking in all functions that accept a list (and new unit tests for each of them) to make sure that the set_expr_type happen in the right place to have the correct type.

@matteojug
Copy link
Contributor Author

matteojug commented Aug 23, 2024

One thing that we may need to double check: is it possible for the same expression (eg, loading a data-var and manipulating it) to be coerced, in different places, to two different types based on the function used (eg, in map)? If so, is it bad?

Since those workarounds basically interpret the list type based on the called function, I was wondering if it could be possible for this to introduce subtle bugs where you define a mistyped function, but this type coercion makes so it's still accepted (eventually reading garbage), but then it's possible to manipulate the original values in the list so that the garbage is no longer garbage but something bad.

It could help to have a clear characterization of the cases that required those workaround in the first place.

@Acaccia
Copy link
Collaborator

Acaccia commented Aug 23, 2024

As much as I know from the testing I did, data-vars seems okay, because you have to set the type at declaration.
But for constants, that is something I was wondering too. I never wrote a test to verify this assumption though.

@aldur aldur assigned aldur and csgui and unassigned aldur Sep 18, 2024
@csgui csgui removed their assignment Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Status: 🆕 New
Development

No branches or pull requests

5 participants