[feat] auto carry for field expression #484
Conversation
INT-2171 Modular field expression support
This makes me want to have a struct similar to |
| } | ||
| } | ||
|
|
||
| // TODO: rethink about how should auto-save work. |
There was a problem hiding this comment.
maybe there is a better way and I am just dumb.
This makes it hard to do, for example x + x because it cannot do 2 mut ref, and we have to clone.
There was a problem hiding this comment.
hmmm the cheat is to just make add a function of two Rc<RefCell<FieldVariable<C>>> with the reasoning that if the only time you mutate is to save, then everything is safe. Only do this if it seems worth it to support x + x.
SymbolicExpr::Add will probably need to be changed too
There was a problem hiding this comment.
probably not worth, as x+x can just be x.int_mul(2) actually
| FieldVariable::<C>::save_if_overflow(self, other, limb_max_fn); | ||
|
|
||
| let limb_max_abs = limb_max_fn(self, other); | ||
| let max_overflow_bits = log2_ceil_usize(limb_max_abs); | ||
| FieldVariable { | ||
| expr: SymbolicExpr::Add(Box::new(self.expr.clone()), Box::new(other.expr.clone())), |
There was a problem hiding this comment.
If at this point, self was auto-saved but other was not, but then later on other is also autosaved, what should the behavior here be?
currently I think it will evaluate using other unsaved since it is a clone, but I guess it is more optimal if it used the autosaved version. I'm not sure which way is easier for the ExpressionBuilder to reason about.
There was a problem hiding this comment.
hmm good point 🤔 but how can this be evaluated with the other being autosaved if that happens later?
There was a problem hiding this comment.
yea I didn't remember/think through how the implementation currently handles this. if it is such that it can't use autosaved other later, let's just forget about it and note that's something that would require a manual save
|
@luffykai ok to merge then |
* add limb info to field variable * auto carry, but carry bit exceeds range check * auto carry and tests
related to INT-2171