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.

Sign up for GitHub

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

Jump to bottom

-3 >> 8 == 0 #49

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

-3 >> 8 == 0 #49

Yaffle opened this issue Jun 6, 2015 · 4 comments

Comments

@Yaffle
Copy link
Contributor

Yaffle commented Jun 6, 2015

in JavaScript -3 >> 8 === -1, in bn.js it is 0.
I think, x >> n should be equal to floor(x / 2**n) for compatibility with JavaScript and some other libraries
@indutny
Copy link
Owner

indutny commented Jul 4, 2015

Hm... maybe we should throw on negative numbers instead? I don't believe there is much use in this compatibility thing, and there are potentially lots of troubles involved with supporting this in the library.

@Yaffle
Copy link
Contributor Author

Yaffle commented Jul 4, 2015

Throwing is fine

@indutny
Copy link
Owner

indutny commented Jul 5, 2015

Hm... it looks like I use this behaviour quite heavily in bn.js. Sorry, but I'm not going to fix it, at least not now.

@indutny indutny closed this as completed Jul 5, 2015
@indutny indutny reopened this Jul 7, 2015
indutny added a commit that referenced this issue Jul 7, 2015
@indutny
bn: iushrn/ushrn, iushln/ushln
98b0c35 
`ishrn`/`ishln` should require positive numbers, `iushrn`/`iushln`
should not give a thing about it.

Fix: #49
indutny added a commit that referenced this issue Jul 7, 2015
@indutny
bn: iushrn/ushrn, iushln/ushln
a976d23 
`ishrn`/`ishln` should require positive numbers, `iushrn`/`iushln`
should not give a thing about it.

Fix: #49
indutny added a commit that referenced this issue Jul 7, 2015
@indutny
bn: iushrn/ushrn, iushln/ushln
e27d6eb 
`ishrn`/`ishln` should require positive numbers, `iushrn`/`iushln`
should not give a thing about it.

Fix: #49
@indutny indutny closed this as completed Jul 7, 2015
@dcousens
Copy link
Contributor

dcousens commented Jul 8, 2015

To clarify for future onlookers, this was fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Yaffle @indutny @dcousens