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

bitwise operators on negative numbers #48

Closed
Yaffle opened this issue Jun 6, 2015 · 4 comments
Closed

bitwise operators on negative numbers #48

Yaffle opened this issue Jun 6, 2015 · 4 comments

Comments

@Yaffle
Copy link
Contributor

Yaffle commented Jun 6, 2015

seems, https://github.com/justmoon/node-bignum throws an error if any a or b is a negative integer for a ^ b, a & b, a | b

The behaviour of this operators for negative integers in other libraries and in JavaScript is as if twos complement arithmetic were used. ( http://www-cs-students.stanford.edu/~tjw/jsbn/ )

I think, you might throw an error while this behaviour is not implemented

@indutny
Copy link
Owner

indutny commented Jul 4, 2015

I'm -1 for implementing it, cc @dcousens

@dcousens
Copy link
Contributor

dcousens commented Jul 7, 2015

@indutny it has always felt that negative numbers have been an afterthought (IMHO) in bn.js, thats not to say they aren't supported, but simply due to the use cases for this library being primarily crypto in a reduced context, they aren't tested/used as heavily (within reason!)
Perhaps we need to decide whether they should be first class citizens or not before we start making the APIs more cross-compatible with standard behaviour?

@indutny
Copy link
Owner

indutny commented Jul 7, 2015

@dcousens I didn't really thought much about them indeed, and the mentioned reasons are correct. I wonder if it might be a good idea to fix it within next major bump.

btw, I think I am convinced that it should be done.

@dcousens
Copy link
Contributor

dcousens commented Jul 7, 2015

@indutny if we are going to continue to support negative numbers (I think we should), then yes, it should be done absolutely.

indutny added a commit that referenced this issue Jul 7, 2015
indutny added a commit that referenced this issue Jul 7, 2015
@indutny indutny closed this as completed Jul 7, 2015
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

No branches or pull requests

3 participants