expr icon indicating copy to clipboard operation
expr copied to clipboard

Cast IntegerNode to float32 in operations like `f32 == 1.5`

Open daisy1754 opened this issue 1 year ago • 11 comments

When you try to compare floating number with floating literal like Number == 12.34, it returns false even when Number is 12.34

Below is code to reproduce https://go.dev/play/p/FFgPkXjIURm

package main

import (
	"fmt"

	"github.com/expr-lang/expr"
)

type Env struct {
	Number float32
}

func main() {
	code := `Number == 12.34`

	program, err := expr.Compile(code, expr.Env(Env{}))
	if err != nil {
		panic(err)
	}

	env := Env{
		Number: 12.34,
	}
	output, err := expr.Run(program, env)
	if err != nil {
		panic(err)
	}

	fmt.Println(output)
}

daisy1754 avatar Mar 20 '24 12:03 daisy1754

This is how float comparison works:

package main

func main() {
	var f32 float32 = 12.34
	var f64 float64 = 12.34
	println(float64(f32) == f64) // false
}

https://go.dev/play/p/XsvrQ66pfrX

antonmedv avatar Apr 15 '24 14:04 antonmedv

@antonmedv I think example I gave is more like below. It is true in golang but false in expr

package main

func main() {
	var f32 float32 = 12.34
	println(f32 == 12.34) // true
}

https://go.dev/play/p/Z9SwZHGSi_J

daisy1754 avatar May 23 '24 16:05 daisy1754

In golang f32 == 12.34, right hand side 12.34 will be interpreted as float32, i.e. float32(12.34). IN Expr 12.34 is a float64.

antonmedv avatar May 24 '24 08:05 antonmedv

well in golang literals are automatically casted to proper type right? in below example both float32 and float64 comparison returns true.

package main

func main() {
	var f32 float32 = 12.34
	var f64 float64 = 12.34
	println(f32 == 12.34) // true
	println(f64 == 12.34) // true
}

daisy1754 avatar May 24 '24 13:05 daisy1754

Yes, type in inherited from left.

antonmedv avatar May 24 '24 14:05 antonmedv

and that is not the case for expr (please refer to my initial example) and that's why I opened issue. I believe Expr internally always cast number to flat64

daisy1754 avatar May 24 '24 16:05 daisy1754

In your example number is float32

antonmedv avatar May 24 '24 17:05 antonmedv

Is this example more clear? I expect all to be true

package main

import (
	"fmt"

	"github.com/expr-lang/expr"
)

type Env struct {
	F32 float32
	F64 float64
}

var env = Env{
	F32: 12.34,
	F64: 12.34,
}

func main() {
	fmt.Printf("f32 expr: %t, go: %t\n", eval(`F32 == 12.34`), env.F32 == 12.34)
	fmt.Printf("f64 expr: %t, go: %t\n", eval(`F64 == 12.34`), env.F64 == 12.34)
}

func eval(code string) bool {
	program, err := expr.Compile(code, expr.Env(Env{}))
	if err != nil {
		panic(err)
	}
	output, err := expr.Run(program, env)
	if err != nil {
		panic(err)
	}
	return output.(bool)
}

result is

f32 expr: false, go: true
f64 expr: true, go: true

https://go.dev/play/p/GH9DSp3YFHF

daisy1754 avatar May 24 '24 18:05 daisy1754

In first example expr does this

F32 == float64(12.34)

which is false.

antonmedv avatar May 25 '24 08:05 antonmedv

yeah I understand internal - that's why I sent out https://github.com/expr-lang/expr/pull/610

Don't you think it's confusing though? As shown above

fmt.Printf("f32 expr: %t, go: %t\n", eval(F32 == 12.34), env.F32 == 12.34)

expr expression is false and golang expression is true in this case

daisy1754 avatar May 25 '24 13:05 daisy1754

Yes! I think we can work on a proper solution for this. The solution in #610 is not correct.

What we need is to cast type to float32 only in case of lhs is float32. This should be done in patcher.

antonmedv avatar May 25 '24 14:05 antonmedv