Deprecated divn and implemented core::ops::Shr
#736
Conversation
hdvanegasm
requested review from
z-tech,
Pratyush and
weikengchen
and removed request for
a team
|
Can someone help me with the "CI / Test against curves (bls12_377)" test? AFAIK, I modified the files that the check is complaining about as you can see in the "Files Changed" tab. Why is it failing? |
|
can you please make two separate pull requests for |
hdvanegasm
changed the title
divn and muln and implemented core::ops::Shr and core::ops::Shl respectivelydivn implemented core::ops::Shr
hdvanegasm
changed the title
divn implemented core::ops::Shrdivn and implemented core::ops::Shr
|
Done! you can consider this as the PR for |
Hi @hdvanegasm, we saw that this check fails because you've deprecated the functions described by the error message. You must change these to the new replacements and the check will pass |
|
Hi @z-tech. Thanks for your reply. Notice that the check is complaining about lines that where changed in this PR. An example of this is presented here: https://github.com/arkworks-rs/algebra/pull/736/files#diff-7bfbb2ccb1c569a529a64dfba848ae83498a02f8f3012fbd7b2c87f0c11c904eR830. You may take a look at the changed files and you will find that all the lines that the check is showing are changed already for the operator |
|
Is it possible that this check is failling because the CI file is pointing to an old version of the repo? Take a look at this: algebra/.github/workflows/ci.yml Line 420 in d9527c8 |
I see. If you remove this does it work? If yes, it is safe to remove we have archived this repo. |
ff/src/biginteger/mod.rs
Outdated
| impl<const N: usize> Shr<u32> for BigInt<N> { | ||
| type Output = Self; | ||
|
|
||
| fn shr(self, mut rhs: u32) -> Self::Output { |
There was a problem hiding this comment.
can you please document here (and in the trait definition) that, differently from the implementation of u8, u32, u64, etc shift right will NOT return an underflow error if the value being shifted is larger than N * sizeof(usize)?
There was a problem hiding this comment.
I think that it should be "(...) will NOT return an underflow error if the value being shifted is larger than N * 64", given that the limb size 64 for the case of BigInt.
I didn't document the trait definition because we are forcing the implementor to specify the shr() function. So, I didn't find an appropriate place to put this message. Let me know if it's fine like it is or if you have a proper place to put this message in the trait definition.
I removed the test against the old `algebra` repository to avoid conflicts in the CI.
I removed the check completely. Let me know if this is what you were looking for. |
Pratyush
force-pushed
the
shl-and-shr
branch
from
a94d9dc to
7473efc
Compare
Description
This PR closes #714. I choose to add
#deprecateannotation todivn()instead of removing it to keep the backward compatibility. Let me know if it's best to remove such old function.