Fix copysign bug #182
Fix copysign bug #182
Conversation
There was a problem hiding this comment.
thanks for the PR.
I have a couple of comments. (see below).
could you also give a slightly more explanatory commit message?
(what has been modified, and, more importantly, why :)
see this document for more details.)
thanks.
exec/num_test.go
Outdated
| vm.pushFloat32(z2) | ||
| vm.funcTable[opcode]() | ||
| if r := vm.popFloat32(); r != z3 { | ||
| op, _ := operators.New(opcode) |
There was a problem hiding this comment.
please add a check for the returned error.
Codecov Report
@@ Coverage Diff @@
## master #182 +/- ##
==========================================
+ Coverage 69.75% 69.77% +0.02%
==========================================
Files 49 49
Lines 5491 5495 +4
==========================================
+ Hits 3830 3834 +4
Misses 1318 1318
Partials 343 343
Continue to review full report at Codecov.
|
|
Thank you for the nice comments. I have added some explanation and changed the code. |
| } | ||
| } | ||
|
|
||
| func TestF64BinOps(t *testing.T) { |
There was a problem hiding this comment.
LGTM.
thanks for finding this issue and fixing it :)
could you send another PR against go-interpreter/license, adding yourself to the list of AUTHORS and CONTRIBUTORS ?
then I'll merge this one.
thanks again.
|
Sent PR for the |
According to the Wasm spec, it seems that f32Copysign and f64Copysign was not correctly implemented. I did some fix and unit tests, but please double check this.