noname icon indicating copy to clipboard operation
noname copied to clipboard

add must_use to the sub, add, mul, etc. functions of the backend trait

Open mimoo opened this issue 10 months ago • 1 comments

right now it's too easy to write code that operates on FieldVar/CellVar which could not lead to constraints, we should use the must_use attribute to have the compiler yell at us when we do this

mimoo avatar Apr 26 '24 10:04 mimoo

Adding this to the backend trait will only yell within the front end layer where calls these backend api, but not at the frontend to frontend calls which wrap these backend calls.

For example, it won't show the warning when the returned var of this sub function isn't constrained via assert_eq:

/// Subtracts two variables, we only support variables that are of length 1.
pub fn sub<B: Backend>(
    compiler: &mut CircuitWriter<B>,
    lhs: &ConstOrCell<B::Field>,
    rhs: &ConstOrCell<B::Field>,
    span: Span,
) -> Var<B::Field> {
    let neg_rhs = neg(compiler, rhs, span);
    add(compiler, lhs, &neg_rhs.cvars[0], span)
}

~~Maybe we need to add an additional layer between the front end api and backend api for composed constraints such as sub. Anything that require composing the backend constraint apis, it should use the middle layer that only calls the backend api.~~

~~So this might mean the additional layer provide a convenient layer for reusability while discouraging the direct front end to front end calls might be difficult to ensure all necessary constraints. The must_use helps us know the potential under-constraints from this middle layer and the backend layer.~~

Maybe we need to think of a way that can check if all the necessary variable are constrained. Or provide a debugging feature that automatically list out all unconstrained variables.

katat avatar Apr 27 '24 02:04 katat