zksecurity / noname

Noname: a programming language to write zkapps
https://zksecurity.github.io/noname/
187 stars 47 forks source link

Functions are not always monomorphized #185

Closed gio54321 closed 1 month ago

gio54321 commented 1 month ago

I was trying to write this simple circuit

fn sum_arr(arr1: [Field; 3], arr2: [Field; 3]) -> [Field; 3] {
    let mut result = [0; 3];
    for idx in 0..3 {
        result[idx] = arr1[idx] + arr2[idx];
    }
    return result;
}

fn main(pub arr: [Field; 3]) -> [Field; 3] {
    let result = sum_arr(arr, arr);
    return result;
}

But, trying to compile it, gives back a panic in circuit writer

thread 'main' panicked at src/circuit_writer/writer.rs:713:26:
expected array

After a bit of debugging, the problem is that the lhs of the assignment

result[idx] = arr1[idx] + arr2[idx];

still has type GenericSizedArray(Field { constant: true }, Concrete(3)), which should not happen since we are in the circuit writer component.

I think the main issue here is that the body of the function sum_arr is not monomorphized, since functions are monomorphized only if needed. Debugging the code, fn_info.sig().require_monomorphization() returns False (which makes sense as the function signature does not contain any generic-sized array), so the function body AST is left as is and passed to the circuit writer component. However the AST contains the GenericSizedArray objects because of the generic sized array result declared inside the function.

katat commented 1 month ago

Great investigation!

Yes, indeed this is a bug. I think the solution can be changing the GenericSizedArray to the concrete type Array when propagating the type during type checking phase.

ExprKind::RepeatedArrayInit { item, size } => {
    let item_node = self
        .compute_type(item, typed_fn_env)?
        .expect("expected a typed item");

    let size_node = self
        .compute_type(size, typed_fn_env)?
        .expect("expected a typed size");

    if is_numeric(&size_node.typ) {
        // use generic array as the size node might include generic parameters or constant vars
        let res = ExprTyInfo::new_anon(TyKind::GenericSizedArray(
            Box::new(item_node.typ),
            Symbolic::parse_expr(size)?,
        ));
        Some(res)
    } else {
        return Err(self.error(ErrorKind::InvalidArraySize, expr.span));
    }
}

After parsing the size expression from Symbolic, we can check if it is a concrete variant. If it is concrete, then instead it should return

let res = ExprTyInfo::new_anon(TyKind::Array(
    Box::new(item_node.typ),
    size,
));
Some(res)
katat commented 1 month ago

created a fix pr: https://github.com/zksecurity/noname/pull/187