-
Notifications
You must be signed in to change notification settings - Fork 48
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
Expand integer trait #489
Expand integer trait #489
Conversation
src/uint/boxed/encoding.rs
Outdated
if err == 0 { | ||
CtOption::new(Self { limbs: res.into() }, Choice::from(1)) | ||
} else { | ||
CtOption::new(Self::zero_with_precision(bits_precision), Choice::from(0)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if err == 0 { | |
CtOption::new(Self { limbs: res.into() }, Choice::from(1)) | |
} else { | |
CtOption::new(Self::zero_with_precision(bits_precision), Choice::from(0)) | |
} | |
CtOption::new(Self { limbs: res.into() }, Choice::from((err == 0) as u8) |
There may be a better way to construct the Choice
, though I don't remember the possible values of err
offhand
// TODO: why not just !self.is_zero()? | ||
self.limbs | ||
.iter() | ||
.fold(Choice::from(0), |acc, limb| acc | limb.is_nonzero().into()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!self.is_zero()
is fine
// TODO: investigate if directly comparing limbs is faster than performing a | ||
// subtraction between limbs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could short-circuit, so that's definitely a yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine aside from addressing some inline TODOs.
There are various allocations that could be optimized away in the boxed implementations but we can do that as a followup.
I'm going to go ahead and land this since it's been so long in the tooth. @xuganyu96 if you'd like to open some followups for those TODOs or optimizations, that'd be great. |
This is a another continuation of #436 after I lost track of where I overrode the master branch's changes.