Skip to content

Conversation

@minad
Copy link
Member

@minad minad commented May 20, 2019

deprecate mp_tc_(and|or|xor) in favor of mp_(and|or|xor)

  • same behavior for positive numbers
  • generalisation for negative numbers, treating them as two complement
  • improve algorithm, iterate once over the digits, manually perform two complement
  • simplify mp_add_d, mp_sub_d
  • functions are safe in case of a==c or b==c
  • renamed mp_tc_div_2d to mp_signed_rsh (signed right shift)

@minad minad requested review from czurnieden and sjaeckel May 20, 2019 19:09
@minad minad changed the title deprecate mp_(and|or|xor) in favor of mp_tc_(and|or|xor) deprecate mp_(and|or|xor) in favor of MUCH IMPROVED mp_tc_(and|or|xor) May 20, 2019
@minad minad mentioned this pull request May 20, 2019
Copy link
Contributor

@czurnieden czurnieden left a comment

Choose a reason for hiding this comment

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

(I would have used the old names but that's more a matter of taste, so…)
Please update the documentation, too.
Otherwise too many complains/feature requests of the form "And where is/can you implement mp_xor?" will come in.
The mp_complement has a different naming scheme, rename it to mp_tc_complement?

@minad
Copy link
Member Author

minad commented May 21, 2019

@czurnieden we could also deprecate mp_tc_x in favor of mp_x with changed behavior for negative mp_ints. I think that would be ok since no one uses mp_x on negative numbers.
I would also prefer that to be honest.

@minad
Copy link
Member Author

minad commented May 21, 2019

@czurnieden I changed the PR here - all bitwise functions are called mp_bit_x now. I think the names are much nicer like this. Also mp_tc_div_2d didn't make much sense as a name. It is a sign extending right shift. Furthermore I updated the docs. Do you agree with these changes?

@minad minad changed the title deprecate mp_(and|or|xor) in favor of MUCH IMPROVED mp_tc_(and|or|xor) deprecate mp_(tc_)?(and|or|xor) in favor of MUCH IMPROVED mp_bit_(and|or|xor) May 21, 2019
@minad minad requested a review from czurnieden May 21, 2019 05:06
@minad minad force-pushed the deprecate-bitwise2 branch 3 times, most recently from bcc3d5a to 4257f42 Compare May 21, 2019 08:10
@sjaeckel
Copy link
Member

I also ask myself if there's even one user who depends on the old functions' behavior of negative numbers... I can't imagine as that was pretty useless, so I'd say we should use the old names!

@minad
Copy link
Member Author

minad commented May 21, 2019

Ok good! I asked myself the same. I will change everything to the old names then. No unnecessary bit in the name. And I must say tc is also pretty ugly and I am a bit sorry for introducing it...

@minad minad force-pushed the deprecate-bitwise2 branch from 4257f42 to fe0c737 Compare May 21, 2019 09:17
@minad
Copy link
Member Author

minad commented May 21, 2019

@sjaeckel @nijtmans updated to old names

@minad minad force-pushed the deprecate-bitwise2 branch from fe0c737 to ce3ed78 Compare May 21, 2019 09:19
@minad minad changed the title deprecate mp_(tc_)?(and|or|xor) in favor of MUCH IMPROVED mp_bit_(and|or|xor) deprecate mp_tc_(and|or|xor) in favor of mp_(and|or|xor) May 21, 2019
This was referenced May 21, 2019
* same behavior for positive numbers
* generalisation for negative numbers, treating them as two complement
* improve algorithm, iterate once over the digits, manually perform two complement
* simplify mp_add_d, mp_sub_d
* functions are safe in case of a==c or b==c
* renamed mp_tc_div_2d to mp_signed_rsh (signed right shift)
@sjaeckel sjaeckel force-pushed the deprecate-bitwise2 branch from b4db9a8 to 1af0de1 Compare May 21, 2019 16:29
@sjaeckel sjaeckel merged commit 3f2d891 into develop May 21, 2019
@sjaeckel sjaeckel deleted the deprecate-bitwise2 branch May 21, 2019 16:52
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.

4 participants