feat: support binary operations for function return type

[katat]

It is quite common to see scenarios like:

fn bigint_add(lhs: [Field; LEN], rhs: [Field; LEN]) -> [Field; LEN + 1]

fn bigint_mul(lhs: [Field; LHS], rhs: [Field; RHS]) -> [Field; LHS + RHS]

fn bigint_mul_nocarry(lhs: [Field; LEN], rhs: [Field; LEN]) -> [Field; LHS * 2 -1]

So this PR is to support the binary operations over symbolic size for function return type.

[mimoo]

in the middle of it, but I'm wondering why the addition of all that code when we can already rely on the parser to warn when there's no parenthesis in an expression of binary operations. I know we discussed that before but it's still not clear to me (again) why we don't just parse an expression there and then convert it later to something we can use

[mimoo]

maybe buff_token.or_else(|| tokens.bump(ctx)).unwrap()?

[katat]

I think the short answer for now is that the signatures parsing is for constructing types, while the expression parsing is not about the type.

[mimoo]

mm I'm not sure I follow, can't you just parse the whole expression at this point with Expr::parse? What is wrong with that approach?

[mimoo]

Looks good tho!

[katat]

Let me try to reason through the differences.

The Ty::parse is for generating this Ty struct:

pub struct Ty {
    pub kind: TyKind,
    pub span: Span,
}

While the Expr::parse is for generating Expr:

pub struct Expr {
    pub node_id: usize,
    pub kind: ExprKind,
    pub span: Span,
}

Because Symbolic is of TyKind::GenericSizedArray, so it is in Ty::parse. Thus it needs to update the Ty::parse to handle the binary operations, although the similar parsing already exists in the Expr::parse.

What your question inspired me to think of is: what if we can use Expr::parse to parse binary expressions in the Ty::parse for the Symbolic. This may reuse the parsing of parenthesis from Expr. Then Symbolic can be constructed from the Expr containing recursive binary operations, which already order the expression node properly based on the parenthesis.

So yeah, looks like the use of Expr::parse would avoid the addition of binary operation parsing in Ty::parse, and simplify the code a lot.

[katat]

yeah, it really simplifies the things a lot!

[mimoo]

nice! (I would even suggest changing the Symbolic::parse function to a TryFrom<Expr> implementation which share the same signature but is more explicit)