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

Value used in lvalue array indexing does not have its type inferred #5606

Closed
nventuro opened this issue Jul 25, 2024 · 5 comments · Fixed by #6744
Closed

Value used in lvalue array indexing does not have its type inferred #5606

nventuro opened this issue Jul 25, 2024 · 5 comments · Fixed by #6744
Assignees
Labels
bug Something isn't working

Comments

@nventuro
Copy link
Contributor

nventuro commented Jul 25, 2024

Aim

Below is a simplified version of a real life function I wrote:

fn push_to_array(array: [Field; 2], new_elem: Field) -> [Field; 3] {
    let mut concatenated = std::unsafe::zeroed();

    for i in 0..array.len() {
        concatenated[i] = array[i];
    }

    concatenated[concatenated.len() - 1] = new_elem;

    concatenated
}

This fails to compile: it resports ambiguous expression errors, and complains about using bracket indexing for concatenated when its type is _ and not array. Typing the value with let mut concatenated: [Field; 3] = std::unsafe::zeroed() causes for the function to compile with no errors. I'm however surprised that I need to specify this type however, since concatenated is the return value of the function: I'd expect its type to be inferred automatically.

This is of course not a huge issue, but I'm wondering whether this is a bug in the type inference mechanism, an oversight, or something that is not supported due to some implementation reason.

@nventuro nventuro added the bug Something isn't working label Jul 25, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jul 25, 2024
@jfecher jfecher changed the title Value used as return type does not have its type inferred Value used in lvalue array indexing does not have its type inferred Jul 25, 2024
@jfecher
Copy link
Contributor

jfecher commented Jul 25, 2024

This is from this chunk of code: https://github.com/noir-lang/noir/blob/master/compiler/noirc_frontend/src/elaborator/statements.rs#L322-L340 where we try to check the array or slice type instead of unifying.

It is a bit awkward to fix since we want to allow arrays and slices so can't unify with either. We can fix it by using try_unify to an array or slice first, then on failure trying to unify again to the other type and only then issuing an error if both fail.

@nventuro
Copy link
Contributor Author

nventuro commented Dec 6, 2024

This issue is now a bit more severe since the LSP does infer the type from the return value, and shows the type hint. However the code fails to compile (expected array got _) unless I manually accept the hint, which seems odd.

@jfecher
Copy link
Contributor

jfecher commented Dec 9, 2024

There's no way for us to infer a "array or slice type" any more so the best we could do here is infer an array type and require explicit annotations if a slice was desired instead. E.g. |a| a[0] would infer |a: [_; _]| and would thus error if a slice was passed in.

@jfecher jfecher self-assigned this Dec 9, 2024
@nventuro
Copy link
Contributor Author

nventuro commented Dec 9, 2024

I'd be fine with either changing that or how the LSP works, it's mostly the fact that they are acting differently that is problematic. E.g. this is highly confusing:

Image

@jfecher
Copy link
Contributor

jfecher commented Dec 9, 2024

@nventuro ah, the LSP compiler error location is a separate issue. The issue there is that the span is wrong. The issue is with concatenated, not array in that case.

Edit: Also, to elaborate why the LSP seemingly differs from the compiler, it actually doesn't, it's just that at the point of the for loop the compiler doesn't know the type yet. After concatenate is later returned and unified against the return type the type becomes known, and the LSP only takes variable types after the compiler finishes everything. Hence why there's a compiler error - the type was unknown _at that point - but the LS still displays the right type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants