noir-lang / noir

Noir is a domain specific language for zero knowledge proofs
https://noir-lang.org
Apache License 2.0
878 stars 190 forks source link

Build dependency graph between variables in name resolution #1122

Closed jfecher closed 8 months ago

jfecher commented 1 year ago

Aim

Trying to write a program

struct ShouldError {
    uses_nested_arrays: [Array; 2],
}

struct Array {
    array: [Field; 3],
}

Expected behavior

Noir should issue an error that nested arrays are currently unimplemented

Bug

The error is only issued sometimes when the program is compiled. This is because on some runs Array is resolved first and when ShouldError is subsequently resolved, it can see all the fields of Array and will error that nested arrays are used. On other runs however, ShouldError is resolved first and sees only an empty struct for Array and thus does not issue an error. In this case, the program continues until monomorphization where it fails with an unreachable assert.

To reproduce

1. 2. 3. 4.

Installation method

None

Nargo version

No response

@noir-lang/noir_wasm version

No response

@noir-lang/barretenberg version

No response

@noir-lang/aztec_backend version

No response

Additional context

No response

Submission Checklist

kevaundray commented 1 year ago

@jfecher what is the status of this issue?

kevaundray commented 1 year ago

Lowest hanging fruit to attempt to fix would be replacing suspicious usages of hashmap with something like indexmap

kevaundray commented 1 year ago

@jfecher - "The above is not enough, we would need a dependency tree of structures, we could also use this in places where globals depend on other globals"

kevaundray commented 1 year ago

Spoke with Jake; he says, ideally name resolution is split into smaller components and we build this dependency tree at the last step to be used in later passes.

jfecher commented 8 months ago

Can we re-tag this E-HIGH? I think it'd require a large rewrite of name resolution to first create the graph then follow its nodes to resolve names in that order.

Edit: Retagged it as "Large" myself. Not sure what the precise difference is between Large and X-Large so I kept it on large.

Savio-Sou commented 8 months ago

precise difference is between Large and X-Large

Choice is all good as long as it feels right. The Size field is used just as a subjective reference for now.