Add bitwise operations support #68
Conversation
dborovcanin
force-pushed
the
add-bitwise
branch
2 times, most recently
from
2a77c7e to
a2d27f3
Compare
Signed-off-by: Dusan Borovcanin <[email protected]>
scr-oath
left a comment
scr-oath
left a comment
There was a problem hiding this comment.
Would you consider adding the Preload to https://github.com/vadv/gopher-lua-libs/blob/master/plugin/preload.go so that anyone who wants to just load everything can do so?
| return | ||
| } | ||
|
|
||
| func intToU32(i int) (uint32, error) { |
There was a problem hiding this comment.
question: Why why not 64 bit?
There was a problem hiding this comment.
Mainly because numbers in Lua 5.1 are 64-bit floating-point, and Go uint64 overflows them so Go 64-bit uint cannot be presented properly in Lua 5.1 floating-point.
There was a problem hiding this comment.
makes sense; it's a little disappointing that it can't use the full size… I wonder if we should suffix methods with 32 to indicate that, but I won't press that and we can consider this resolved
There was a problem hiding this comment.
I can add a suffix if you insist. I was considering that, but I didn't do it because it's impossible to have <operation>u64 and u8 or u16 can be handled with the existing version. Maybe we can add a suffix in the future if we support different versions (possibly even u64), but that is unlikely to happen.
Signed-off-by: Dusan Borovcanin <[email protected]>
Signed-off-by: Dusan Borovcanin <[email protected]>
Signed-off-by: Dusan Borovcanin <[email protected]>
Signed-off-by: Dusan Borovcanin <[email protected]>
Signed-off-by: Dusan Borovcanin <[email protected]>
Signed-off-by: Dusan Borovcanin <[email protected]>
Signed-off-by: Dusan Borovcanin <[email protected]>
Signed-off-by: Dusan Borovcanin <[email protected]>
| local tests = { | ||
| { | ||
| input1 = -3, | ||
| input2 = 23, | ||
| expected = nil, | ||
| err = "cannot convert negative int -3 to uint32", | ||
| }, | ||
| { | ||
| input1 = 4294967296, | ||
| input2 = 23, | ||
| expected = nil, | ||
| err = "int 4294967296 overflows uint32", | ||
| }, | ||
| { | ||
| input1 = 1, | ||
| input2 = 0, | ||
| expected = 0, | ||
| }, | ||
| { | ||
| input1 = 111, | ||
| input2 = 222, | ||
| expected = 78, | ||
| } | ||
| } |
There was a problem hiding this comment.
praise: I like this style - I was thinking of doing it - Github copilot did the positional thing for me so I left it, but associative arrays are more descriptive!
There was a problem hiding this comment.
Thanks! I completely missed your PR on my branch, so I did it this way, similarly to the tests you linked in the previous comments (there's a bit of repetition, but I hope that's acceptable).
| return | ||
| } | ||
|
|
||
| func intToU32(i int) (uint32, error) { |
There was a problem hiding this comment.
makes sense; it's a little disappointing that it can't use the full size… I wonder if we should suffix methods with 32 to indicate that, but I won't press that and we can consider this resolved
|
Did you have more you wanted to add? I can merge this, but as you said that it wasn't ready before, just wanted to give you another chance to self-review and/or update before I merge… just give me the go and I'll merge/cut a release. |
This is ready unless there are further comments. I could add a few more operations (e.g., |
3 participants
Add support for bitwise operations:
and,or,xor,not,lshift,rshift.