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

Hint ite statements #266

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

0xnullifier
Copy link
Contributor

@katat I think I could not explain things in the issue so I have raised a draft pull request. For the example of if_else.no taken in the pr it works as the blocks early returns but if I change the example to something like this:

hint fn ite(xx: Field , arr: [Field;LEN]) -> Field {
    let mut var = 0;
    if xx == 10 {
        var = xx;
        for idx in 0..LEN {
            var = var + arr[idx];
        }
        var = xx * var;
    } else {
       var = xx * xx;
    }
    return var;
}

this causes the then_branch return type to panic. So how can I support these type of statements? moreover what about only if statements?

Copy link
Collaborator

@katat katat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason it panics is the example you provided returns none for the ITE branches.

Let's first agree on the grammar on this enhanced ITE statement.

I would suggest to still align with the grammar supported in the constraint code:

let res = if cond {t1} else {t2};

In hint function for now, we would just add the support of return statement.

In your example, we would instead enforce it to be:

hint fn ite(xx: Field , arr: [Field;LEN]) -> Field {
    let mut var = 0;
    let res = if xx == 10 {
        var = xx;
        for idx in 0..LEN {
            var = var + arr[idx];
        }
        xx * var // align with the existing grammar
    } else {
       return xx * xx // or it returns
    }
    return res;
}

This way aligns with the existing grammar in constraint mode, while avoiding adding too much complexity for this enhancement at this stage.

.value(self, fn_env)
.cvars[0]
.clone();
let ret = self.compile_block(fn_env, then_branch)?.unwrap().cvars[0].clone();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why then_branch block isn't handled the same way as the else_branch block?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't the fn_env.nest() and fn_env.pop() be called for each compile_block?

@0xnullifier
Copy link
Contributor Author

@katat hey sorry for the delay I am a little busy with some work this week. I will pick this up again after the weekend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants