expr icon indicating copy to clipboard operation
expr copied to clipboard

Document/review arithmetic conversion rules

Open TheCount opened this issue 5 years ago • 4 comments

The documentation doesn't say much about arithmetic conversion (or I couldn't find it). I wrote a little test program:

Type tester
package main

import (
        "fmt"
        "log"
        "github.com/antonmedv/expr"
)

func run(code string, env map[string]interface{}) {
        prog, err := expr.Compile(code, expr.Env(env))
        if err != nil {
                log.Fatalf("compile '%s': %s", code, err)
        }
        result, err := expr.Run(prog, env)
        if err != nil {
                log.Fatalf("run '%s': %s", code, err)
        }
        fmt.Printf("%s: %v (%T)\n", code, result, result)
}

func main() {
        test := map[string]interface{}{
                "Int": -1234567890,
                "Int8": int8(-123),
                "Int16": int16(-12345),
                "Int32": int32(-1234567890),
                "Int64": int64(-12345678901),
                "Uint": uint(3456789012),
                "Uint8": uint8(234),
                "Uint16": uint16(34567),
                "Uint32": uint32(3456789012),
                "Uint64": uint64(12345678901234567890),
                "Float32": float32(1000003),
                "Float64": float64(1000000007),
        }
        run(`0`, test)
        run(`0.0`, test)
        run(`Int`, test)
        run(`Int8`, test)
        run(`Int16`, test)
        run(`Int32`, test)
        run(`Int64`, test)
        run(`Uint`, test)
        run(`Uint8`, test)
        run(`Uint16`, test)
        run(`Uint32`, test)
        run(`Uint64`, test)
        run(`Float32`, test)
        run(`Float64`, test)
        run(`Int+0`, test)
        run(`Int8+0`, test)
        run(`Int16+0`, test)
        run(`Int32+0`, test)
        run(`Int64+0`, test)
        run(`Uint+0`, test)
        run(`Uint8+0`, test)
        run(`Uint16+0`, test)
        run(`Uint32+0`, test)
        run(`Uint64+0`, test)
        run(`Float32+0`, test)
        run(`Float64+0`, test)
        run(`Float32+Float64`, test)
}

It gives me the following output:

Output
0: 0 (int)
0.0: 0 (float64)
Int: -1234567890 (int)
Int8: -123 (int8)
Int16: -12345 (int16)
Int32: -1234567890 (int32)
Int64: -12345678901 (int64)
Uint: 3456789012 (uint)
Uint8: 234 (uint8)
Uint16: 34567 (uint16)
Uint32: 3456789012 (uint32)
Uint64: 12345678901234567890 (uint64)
Float32: 1.000003e+06 (float32)
Float64: 1.000000007e+09 (float64)
Int+0: -1234567890 (int)
Int8+0: -123 (int8)
Int16+0: -12345 (int16)
Int32+0: -1234567890 (int32)
Int64+0: -12345678901 (int64)
Uint+0: -838178284 (int)
Uint8+0: 234 (int)
Uint16+0: 34567 (int)
Uint32+0: -838178284 (int)
Uint64+0: -350287150 (int)
Float32+0: 1.000003e+06 (float32)
Float64+0: 1.000000007e+09 (float64)
Float32+Float64: 1.00100001e+09 (float64)

So the default arithmetic types appear to be int and float64.

For floats, expr appears to keep the float32 type for "pure" float32 expressions, upgrading to float64 for non-pure expressions when necessary. This seems reasonable and expected to me, but should be documented.

~~For the integers, the behaviour is weirder. For "primitive" expressions (no actual arithmetic, just the value), the type is kept except for uint, which for some reason comes out as int. This looks like a bug to me.~~

Even for "pure" expressions, the unsigned integer types are invariably converted to int, while signed integer types are left alone. For systems on which int is 32 bit wide (as is the case here), this can lead to "wrong" results for uint, uint32, and uint64 expressions, depending on the value. For very large values (≥ 2⁶³) this is perhaps expected, but for reasonably large integers (between 2³¹ and 2⁶³, which can be correctly represented by both int64 and uint64), it feels wrong that the latter choice leads to a bad conversion while the former does not.

Maybe the simplest solution would be to treat all integers as int64 and document this fact. This would also relieve the user of the burden to perform complicated type conversions on the result.

Edit: Fixed test program. Sorry.

TheCount avatar Nov 16 '20 12:11 TheCount

OK, regarding uint,I made a mistake in the test program. Sorry, I'll fix the example.

TheCount avatar Nov 16 '20 15:11 TheCount

OK, regarding uint,I made a mistake in the test program. Sorry, I'll fix the example.

Fixed. Again, sorry.

TheCount avatar Nov 16 '20 15:11 TheCount

Yes, you are right. This should be documented. Idea is to convert to arithmetic operations to closest type which will fit. But this is wrong for all uint.

I think int and uint should be separated in expr and no automatic type conversion take place.

antonmedv avatar Nov 16 '20 16:11 antonmedv

Some builtints as int and uint may be introduced.

antonmedv avatar Nov 16 '20 16:11 antonmedv