Code Monkey home page Code Monkey logo

Comments (31)

andig avatar andig commented on September 12, 2024 1
=== RUN   Test_issue457_toInt32_3
abs: 2.147483648e+09
iv: 2.147483648e+09
iv32: 2.147483648e+09
res1: -2147483648
--- FAIL: Test_issue457_toInt32_3 (0.00s)

from otto.

andig avatar andig commented on September 12, 2024 1

Passing with latest commit:

commit a4c7202cfd14ce01451d87b48c0bce0620431b86 (HEAD -> fix/number-to-ints, origin/fix/number-to-ints)
Author: Steven Hartland <[email protected]>
Date:   Wed Nov 30 16:58:51 2022 +0000

    fix: use int64 not uint64 for uint cases

    Try using int64 instead of uint64 for uint cases.

from otto.

stevenh avatar stevenh commented on September 12, 2024

Latest master is passing for me:

go test -run=TestBinaryShiftOperation -v
=== RUN   TestBinaryShiftOperation
--- PASS: TestBinaryShiftOperation (0.00s)
PASS
ok      github.com/robertkrimen/otto    0.004s

Do you happen to testing on a 32bit architecture?

from otto.

andig avatar andig commented on September 12, 2024

Fails on 64bit OSX:

uname -a
Darwin SkyNetM1.local 22.1.0 Darwin Kernel Version 22.1.0: Sun Oct  9 20:14:30 PDT 2022; root:xnu-8792.41.9~2/RELEASE_ARM64_T8103 arm64


go test -run=TestBinaryShiftOperation -v
Alias tip: got -run=TestBinaryShiftOperation -v
=== RUN   TestBinaryShiftOperation
~~~ FAIL: (Terst)
	runtime_test.go:592:
		FAIL (==)
		     got: 1073741823 (int32)
		expected: -1073741824 (int)
--- FAIL: TestBinaryShiftOperation (0.00s)
FAIL
exit status 1
FAIL	github.com/robertkrimen/otto	0.139s

from otto.

stevenh avatar stevenh commented on September 12, 2024

Hmm, what does the following report:

package main

import (
	"testing"

	"github.com/robertkrimen/otto"
	"github.com/stretchr/testify/require"
)

func Test_issue457(t *testing.T) {
	vm := otto.New()
	val, err := vm.Run(`
		high = (1 << 30) - 1 + (1 << 30);
		low = -high - 1;
		vwx = low >> 1;
		[high, low, vwx];
	`)
	require.NoError(t, err)
	exp, err := val.Export()
	require.NoError(t, err)
	require.Equal(t, []interface{}{float64(2147483647), float64(-2147483648), int32(-1073741824)}, exp)

	t.Log(exp)
}

from otto.

andig avatar andig commented on September 12, 2024
=== RUN   Test_issue457
    issue2_test.go:20:
        	Error Trace:	/Users/andig/htdocs/otto/issue2_test.go:20
        	Error:      	Not equal:
        	            	expected: "2147483647,-2147483648,-1073741824"
        	            	actual  : "2147483647,-2147483648,1073741823"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-2147483647,-2147483648,-1073741824
        	            	+2147483647,-2147483648,1073741823
        	Test:       	Test_issue457
--- FAIL: Test_issue457 (0.00s)

from otto.

stevenh avatar stevenh commented on September 12, 2024

Well that's strange, what about:

package main

import (
	"fmt"
	"testing"

	"github.com/stretchr/testify/require"
)

func Test_issue457(t *testing.T) {
	v := int32(-2147483648) >> 1
	fmt.Printf("%T = %v\n", v, v)
	require.Equal(t, int32(-1073741824), v)
}

from otto.

andig avatar andig commented on September 12, 2024
got -v issue2_test.go
=== RUN   Test_issue457
int32 = -1073741824
--- PASS: Test_issue457 (0.00s)
PASS
ok  	command-line-arguments	0.254s

from otto.

stevenh avatar stevenh commented on September 12, 2024

Ok could you apply this patch and rerun the the otto Test_issue457.
evaluate.patch.txt

from otto.

andig avatar andig commented on September 12, 2024
=== RUN   Test_issue457
shift1: otto.Value{kind:2, value:-2.147483648e+09}, otto.Value{kind:2, value:1}
shift2: 2147483647, 1
shift3: 1073741823
    issue2_test.go:20:
        	Error Trace:	issue2_test.go:20
        	Error:      	Not equal:
        	            	expected: "2147483647,-2147483648,-1073741824"
        	            	actual  : "2147483647,-2147483648,1073741823"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-2147483647,-2147483648,-1073741824
        	            	+2147483647,-2147483648,1073741823
        	Test:       	Test_issue457
--- FAIL: Test_issue457 (0.00s)

from otto.

stevenh avatar stevenh commented on September 12, 2024

Oh wow, I see:

shift1: otto.Value{kind:2, value:-2.147483648e+09}, otto.Value{kind:2, value:1}
shift2: -2147483648, 1
shift3: -1073741824

This means that converting -2.147483648e+09 to int32 using toInt32(leftValue) is loosing the sign.

Here's an instrumented version, if you could run this.

func Test_issue457_toInt32(t *testing.T) {
	val := int32(-2147483648)
	fval := float64(val)
	fmt.Printf("val: %v, fval: %v\n", val, fval)

	value := Value{kind: 2, value: fval}
	floatValue := value.float64()
	remainder := math.Mod(floatValue, float_2_32)
	fmt.Printf("rem1: %v, float_2_32: %v\n", remainder, float_2_32)
	if remainder > 0 {
		remainder = math.Floor(remainder)
		fmt.Printf("rem2: %v\n", remainder)
	} else {
		remainder = math.Ceil(remainder) + float_2_32
		fmt.Printf("rem3: %v\n", remainder)
	}

	fmt.Printf("rem4: %v, float_2_31: %v\n", remainder, float_2_31)
	if remainder > float_2_31 {
		require.Equal(t, val, int32(remainder-float_2_32))
	}
	require.Equal(t, val, int32(remainder))
}

from otto.

andig avatar andig commented on September 12, 2024
=== RUN   Test_issue457
shift1: otto.Value{kind:2, value:-2.147483648e+09}, otto.Value{kind:2, value:1}
shift2: 2147483647, 1
shift3: 1073741823
    issue2_test.go:22:
        	Error Trace:	issue2_test.go:22
        	Error:      	Not equal:
        	            	expected: "2147483647,-2147483648,-1073741824"
        	            	actual  : "2147483647,-2147483648,1073741823"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-2147483647,-2147483648,-1073741824
        	            	+2147483647,-2147483648,1073741823
        	Test:       	Test_issue457
--- FAIL: Test_issue457 (0.00s)
=== RUN   Test_issue457_toInt32
val: -2147483648, fval: -2.147483648e+09
rem1: -2.147483648e+09, float_2_32: 4.294967296e+09
rem3: 2.147483648e+09
rem4: 2.147483648e+09, float_2_31: 2.147483648e+09
    issue2_test.go:47:
        	Error Trace:	issue2_test.go:47
        	Error:      	Not equal:
        	            	expected: -2147483648
        	            	actual  : 2147483647
        	Test:       	Test_issue457_toInt32
--- FAIL: Test_issue457_toInt32 (0.00s)

What I don't get: why aren't you seeing this error on CI? The runners are 64bits afaik. What is different between those and my local machine?

from otto.

stevenh avatar stevenh commented on September 12, 2024

My machine here is 64bit but x86_64, I thought this was a 32bit arch issue but it could be something more subtle to do with internal architecture.

Looks like you might be on an m1 mac as the uname stated arm64?

Hopefully this final version will uncover the issue:

func Test_issue457_toInt32_2(t *testing.T) {
	val := int32(-2147483648)
	fval := float64(val)
	fmt.Printf("maxInt32: %v minInt32: %v\n", math.MaxInt32, math.MinInt32)
	fmt.Printf("float_2_32: %v, float_2_31: %v\n", int64(float_2_32), int64(float_2_31))
	fmt.Printf("val: %v, fval: %v\n", val, fval)

	value := Value{kind: 2, value: fval}
	floatValue := value.float64()
	remainder := math.Mod(floatValue, float_2_32)
	fmt.Printf("rem1: %v, float_2_32: %v\n", remainder, float_2_32)
	if remainder > 0 {
		remainder = math.Floor(remainder)
		fmt.Printf("rem2: %v\n", remainder)
	} else {
		remainder = math.Ceil(remainder) + float_2_32
		fmt.Printf("rem3: %v\n", remainder)
	}

	fmt.Printf("rem4: %v, float_2_31: %v\n", remainder, float_2_31)
	if remainder > float_2_31 {
		fmt.Printf("out1: %v, %T, %v\n", remainder-float_2_32, remainder-float_2_32, int32(remainder-float_2_32))
		require.Equal(t, val, int32(remainder-float_2_32), "out1")
	}
	fmt.Printf("out2: %v, %T, %v\n", remainder, remainder, int32(remainder))
	require.Equal(t, val, int32(remainder), "out2")
	t.FailNow()
}

from otto.

stevenh avatar stevenh commented on September 12, 2024

We could be seeing something similar to this issue

from otto.

stevenh avatar stevenh commented on September 12, 2024

Out of interest if you paste this into the console in chrome or other browser what does the following show:

high = (1 << 30) - 1 + (1 << 30);
low = -high - 1;
vwx = low >> 1;
[high, low, vwx];

from otto.

andig avatar andig commented on September 12, 2024

My machine here is 64bit but x86_64, I thought this was a 32bit arch issue but it could be something more subtle to do with internal architecture.

Interesting. Had a quick look at the code and didn't see anything obvious. Still need to look at the issue.

Now:

=== RUN   Test_issue457_toInt32_2
maxInt32: 2147483647 minInt32: -2147483648
float_2_32: 4294967296, float_2_31: 2147483648
val: -2147483648, fval: -2.147483648e+09
rem1: -2.147483648e+09, float_2_32: 4.294967296e+09
rem3: 2.147483648e+09
rem4: 2.147483648e+09, float_2_31: 2.147483648e+09
out2: 2.147483648e+09, float64, 2147483647
    issue2_test.go:75:
        	Error Trace:	issue2_test.go:75
        	Error:      	Not equal:
        	            	expected: -2147483648
        	            	actual  : 2147483647
        	Test:       	Test_issue457_toInt32_2
        	Messages:   	out2
--- FAIL: Test_issue457_toInt32_2 (0.00s)

And:

Screenshot 2022-11-30 at 13 07 14

from otto.

stevenh avatar stevenh commented on September 12, 2024

Ok what about this:

func Test_issue457_toInt32_3(t *testing.T) {
	val := int32(-2147483648)
	floatValue := float64(val)

	if math.IsNaN(floatValue) || math.IsInf(floatValue, 0) || floatValue == 0 {
		fmt.Println("zero")
		t.FailNow()
		return
	}

	abs := math.Abs(floatValue)
	fmt.Printf("abs: %v\n", abs)
	iv := math.Floor(abs)
	fmt.Printf("iv: %v\n", iv)
	iv32 := math.Mod(iv, float_2_32)
	fmt.Printf("iv32: %v\n", iv32)
	if iv32 >= float_2_31 {
		res := int32(iv32 - float_2_32)
		fmt.Printf("res1: %v\n", res)
		t.FailNow()
		return
	}
	fmt.Printf("res2: %v\n", int32(iv32))
	t.FailNow()
}

from otto.

andig avatar andig commented on September 12, 2024

Maybe I could help: what is the conversion you're trying to achieve? Float to int32? What is the desired over/underflow behaviour? Then I could see if I can code something that works here.

Also see https://stackoverflow.com/questions/8022389/convert-a-float64-to-an-int-in-go:

Such casts have a problem in Go that can be unexpected (at least if you come from Java): "In all non-constant conversions involving floating-point or complex values, if the result type cannot represent the value the conversion succeeds but the result value is implementation-dependent." (golang.org/ref/spec#Conversions). So if you use a very large value effectively as an infinity, and you convert it into an int, the result is undefined (unlike Java, where it is the maximal value of the integer type). –
Jaan
May 27, 2016 at 14:37

from otto.

stevenh avatar stevenh commented on September 12, 2024

Can you try this #474

from otto.

andig avatar andig commented on September 12, 2024

Probably offtopic, but anyway:

func TestSimple(t *testing.T) {
	val := int32(-2147483648)
	floatVal := float64(val)

	var res int32

	switch {
	case floatVal > float64(math.MaxInt32):
		res = math.MaxInt32
	case floatVal < float64(math.MinInt32):
		res = math.MinInt32
	default:
		res = int32(floatVal)
	}

	t.Log(res)
}

from otto.

stevenh avatar stevenh commented on September 12, 2024

Needs more work

from otto.

stevenh avatar stevenh commented on September 12, 2024

Ok try that #474 now please @andig.

If this works I have one more change to try, which makes the functions really simple, but wanted to try this first.

from otto.

andig avatar andig commented on September 12, 2024

I guess you've fixed the original one? Here's my entire output:

=== RUN   Test_issue457
    issue2_test.go:23: 2147483647,-2147483648,-1073741824
--- PASS: Test_issue457 (0.00s)
=== RUN   Test_issue457_toInt32
val: -2147483648, fval: -2.147483648e+09
rem1: -2.147483648e+09, float_2_32: 4.294967296e+09
rem3: 2.147483648e+09
rem4: 2.147483648e+09, float_2_31: 2.147483648e+09
    issue2_test.go:47:
        	Error Trace:	/Users/andig/htdocs/otto/issue2_test.go:47
        	Error:      	Not equal:
        	            	expected: -2147483648
        	            	actual  : 2147483647
        	Test:       	Test_issue457_toInt32
--- FAIL: Test_issue457_toInt32 (0.00s)
=== RUN   Test_issue457_toInt32_2
maxInt32: 2147483647 minInt32: -2147483648
float_2_32: 4294967296, float_2_31: 2147483648
val: -2147483648, fval: -2.147483648e+09
rem1: -2.147483648e+09, float_2_32: 4.294967296e+09
rem3: 2.147483648e+09
rem4: 2.147483648e+09, float_2_31: 2.147483648e+09
out2: 2.147483648e+09, float64, 2147483647
    issue2_test.go:75:
        	Error Trace:	/Users/andig/htdocs/otto/issue2_test.go:75
        	Error:      	Not equal:
        	            	expected: -2147483648
        	            	actual  : 2147483647
        	Test:       	Test_issue457_toInt32_2
        	Messages:   	out2
--- FAIL: Test_issue457_toInt32_2 (0.00s)
=== RUN   Test_issue457_toInt32_3
abs: 2.147483648e+09
iv: 2.147483648e+09
iv32: 2.147483648e+09
res1: -2147483648
--- FAIL: Test_issue457_toInt32_3 (0.00s)

from otto.

stevenh avatar stevenh commented on September 12, 2024

Ok so hopefully one last go, just pushed another update. If you delete all the addon tests you should see a clean pass on go test ./... from the root of the repo.

from otto.

andig avatar andig commented on September 12, 2024

You're not gonna like it:

got ./...
~~~ FAIL: (Terst)
	issue_test.go:639:
		FAIL (==)
		     got: 000000000626540e598e480000000000000000004048391f4a1929274d7c695c (*otto._object=ptr)
		expected: aa747c502a898200f9e4fa21bac68136f886a0e27aec70ba06daf2e2a5cb5597
--- FAIL: Test_issue87 (0.01s)
panic triggered: test
interrupt
interrupt
~~~ FAIL: (Terst)
	runtime_test.go:593:
		FAIL (==)
		     got: 0 (uint32)
		expected: 1073741824 (int)
--- FAIL: TestBinaryShiftOperation (0.00s)
~~~ FAIL: (Terst)
	string_test.go:67:
		FAIL (==)
		     got: 0 (uint16)
		expected: 65535 (int)
--- FAIL: TestString_fromCharCode (0.00s)
~~~ FAIL: (Terst)
	value_test.go:174:
		FAIL (==)
		     got: 0 (uint32)
		expected: 2147483647 (uint32)
	value_test.go:174:
		FAIL (==)
		     got: 0 (uint32)
		expected: 4294967295 (uint32)
	value_test.go:174:
		FAIL (==)
		     got: 0 (uint32)
		expected: 1 (uint32)
--- FAIL: Test_toUint32 (0.00s)
~~~ FAIL: (Terst)
	value_test.go:196:
		FAIL (==)
		     got: 0 (uint16)
		expected: 65535 (uint16)
	value_test.go:196:
		FAIL (==)
		     got: 0 (uint16)
		expected: 65535 (uint16)
	value_test.go:196:
		FAIL (==)
		     got: 0 (uint16)
		expected: 1 (uint16)
--- FAIL: Test_toUint16 (0.00s)
FAIL
FAIL	github.com/robertkrimen/otto	1.214s
ok  	github.com/robertkrimen/otto/ast	0.256s
?   	github.com/robertkrimen/otto/dbg	[no test files]
?   	github.com/robertkrimen/otto/file	[no test files]
?   	github.com/robertkrimen/otto/otto	[no test files]
ok  	github.com/robertkrimen/otto/parser	0.175s
?   	github.com/robertkrimen/otto/registry	[no test files]
?   	github.com/robertkrimen/otto/repl	[no test files]
?   	github.com/robertkrimen/otto/terst	[no test files]
?   	github.com/robertkrimen/otto/test	[no test files]
?   	github.com/robertkrimen/otto/token	[no test files]
?   	github.com/robertkrimen/otto/underscore	[no test files]
FAIL

at

commit 3376c332e8f9c4e21cc2b41b29c85635ff7b7d7b (HEAD -> fix/number-to-ints, origin/fix/number-to-ints)
Author: Steven Hartland <[email protected]>
Date:   Wed Nov 30 15:17:41 2022 +0000

    fix: leverage go casts for integer conversion

    Leverage the power of go casts to simplify the conversion from float64
    to integer types.

 value_number.go | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

from otto.

stevenh avatar stevenh commented on September 12, 2024

To confirm the previous commit was fine?

from otto.

stevenh avatar stevenh commented on September 12, 2024

I wonder if it always needs int64 cast to ensure signed, just pushed a change to that effect, see if that fixes those uintXX failures?

from otto.

andig avatar andig commented on September 12, 2024

No. Same result, but we also didn't test these before:

got ./...
~~~ FAIL: (Terst)
	issue_test.go:639:
		FAIL (==)
		     got: 000000000626540e598e480000000000000000004048391f4a1929274d7c695c (*otto._object=ptr)
		expected: aa747c502a898200f9e4fa21bac68136f886a0e27aec70ba06daf2e2a5cb5597
--- FAIL: Test_issue87 (0.01s)
panic triggered: test
interrupt
interrupt
~~~ FAIL: (Terst)
	runtime_test.go:593:
		FAIL (==)
		     got: 0 (uint32)
		expected: 1073741824 (int)
--- FAIL: TestBinaryShiftOperation (0.00s)
~~~ FAIL: (Terst)
	string_test.go:67:
		FAIL (==)
		     got: 0 (uint16)
		expected: 65535 (int)
--- FAIL: TestString_fromCharCode (0.00s)
~~~ FAIL: (Terst)
	value_test.go:174:
		FAIL (==)
		     got: 0 (uint32)
		expected: 2147483647 (uint32)
	value_test.go:174:
		FAIL (==)
		     got: 0 (uint32)
		expected: 4294967295 (uint32)
	value_test.go:174:
		FAIL (==)
		     got: 0 (uint32)
		expected: 1 (uint32)
--- FAIL: Test_toUint32 (0.00s)
~~~ FAIL: (Terst)
	value_test.go:196:
		FAIL (==)
		     got: 0 (uint16)
		expected: 65535 (uint16)
	value_test.go:196:
		FAIL (==)
		     got: 0 (uint16)
		expected: 65535 (uint16)
	value_test.go:196:
		FAIL (==)
		     got: 0 (uint16)
		expected: 1 (uint16)
--- FAIL: Test_toUint16 (0.00s)
FAIL
FAIL	github.com/robertkrimen/otto	1.101s
ok  	github.com/robertkrimen/otto/ast	(cached)
?   	github.com/robertkrimen/otto/dbg	[no test files]
?   	github.com/robertkrimen/otto/file	[no test files]
?   	github.com/robertkrimen/otto/otto	[no test files]
ok  	github.com/robertkrimen/otto/parser	(cached)
?   	github.com/robertkrimen/otto/registry	[no test files]
?   	github.com/robertkrimen/otto/repl	[no test files]
?   	github.com/robertkrimen/otto/terst	[no test files]
?   	github.com/robertkrimen/otto/test	[no test files]
?   	github.com/robertkrimen/otto/token	[no test files]
?   	github.com/robertkrimen/otto/underscore	[no test files]
FAIL

at

commit f062ffba5edb28c8e1526ef15a6f1207ea51c83c (HEAD -> fix/number-to-ints)
Author: Steven Hartland <[email protected]>
Date:   Wed Nov 30 15:11:54 2022 +0000

    fix: sign and overflow

    Fix sign and overflow / underflow cast issues.

from otto.

andig avatar andig commented on September 12, 2024

I've only run the -run Test_issue457 line before.

from otto.

stevenh avatar stevenh commented on September 12, 2024

Big thanks for all your help on this one @andig!

from otto.

andig avatar andig commented on September 12, 2024

Thanks for figuring it out- I know how tedious that is when you can't test yourself...

from otto.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.