traefik / yaegi

Yaegi is Another Elegant Go Interpreter
https://pkg.go.dev/github.com/traefik/yaegi
Apache License 2.0
6.94k stars 343 forks source link

double assignment + closure, in a for loop, reuses the same variable, instead of assigning to a new one. #1497

Closed mpl closed 1 year ago

mpl commented 1 year ago

The following program sample.go triggers an unexpected result

package main

import (
    "bufio"
    "flag"
    "fmt"
    "strconv"
    "strings"
)

var debug = flag.Bool("debug", false, `debug mode`)

type monkey struct {
    items        []int
    operation    func(int) int
    test         func(int) bool
    monkeyTrue   int
    monkeyFalse  int
    inspectCount int
}

func parseOperation(operation string) func(int) int {
    parts := strings.Split(operation, " ")
    self := false
    if parts[1] == "old" {
        self = true
    }
    operand, _ := strconv.Atoi(parts[1])
    sign := string(operation[0])
    if sign == "+" {
        return func(i int) int {
            if self {
                return i + i
            }
            return i + operand
        }
    }
    if sign == "*" {
        return func(i int) int {
            if self {
                return i * i
            }
            return i * operand
        }
    }
    panic(fmt.Sprintf("unsupported operation: %s", operation))
}

var input = `Monkey 0:
  Starting items: 79, 98
  Operation: new = old * 19
  Test: divisible by 23
    If true: throw to monkey 2
    If false: throw to monkey 3

Monkey 1:
  Starting items: 54, 65, 75, 74
  Operation: new = old + 6
  Test: divisible by 19
    If true: throw to monkey 2
    If false: throw to monkey 0

Monkey 2:
  Starting items: 79, 60, 97
  Operation: new = old * old
  Test: divisible by 13
    If true: throw to monkey 1
    If false: throw to monkey 3

Monkey 3:
  Starting items: 74
  Operation: new = old + 3
  Test: divisible by 17
    If true: throw to monkey 0
    If false: throw to monkey 1
`

func main() {
    flag.Parse()
    /*
        f, err := os.Open("./input.txt")
        if err != nil {
            log.Fatal(err)
        }
        defer f.Close()
    */

    f := strings.NewReader(input)
    sc := bufio.NewScanner(f)

    var monkeys []*monkey

    debugCount := 0
    for sc.Scan() {
        if *debug {
            if debugCount > 34 {
                break
            }
        }
        debugCount++

        l := sc.Text()
        if !strings.HasPrefix(l, "Monkey") {
            panic("BIM")
        }
        kong := monkey{}

        sc.Scan()
        l = sc.Text()
        items := strings.Split(strings.TrimPrefix(l, "  Starting items: "), ", ")
        for _, v := range items {
            item, _ := strconv.Atoi(v)
            kong.items = append(kong.items, item)
        }

        sc.Scan()
        l = sc.Text()
        operation := strings.TrimPrefix(l, "  Operation: new = old ")
        kong.operation = parseOperation(operation)

        sc.Scan()
        l = sc.Text()
        if !strings.HasPrefix(l, "  Test: divisible by ") {
            panic(fmt.Sprintf("unsupported test: %s", l))
        }
        divisor, _ := strconv.Atoi(strings.TrimPrefix(l, "  Test: divisible by "))
        if *debug {
            println(len(monkeys), "DIVISOR: ", divisor)
        }
        // TODO: we should defer, and make it directly return the monkey destination
        kong.test = func(i int) bool {
            return i%divisor == 0
        }

        sc.Scan()
        l = sc.Text()
        kong.monkeyTrue, _ = strconv.Atoi(strings.TrimPrefix(l, "    If true: throw to monkey "))
        if *debug {
            println(len(monkeys), "MONKEYTRUE: ", kong.monkeyTrue)
        }

        sc.Scan()
        l = sc.Text()
        kong.monkeyFalse, _ = strconv.Atoi(strings.TrimPrefix(l, "    If false: throw to monkey "))
        if *debug {
            println(len(monkeys), "MONKEYFALSE: ", kong.monkeyFalse)
        }

        monkeys = append(monkeys, &kong)

        sc.Scan()
    }

    for i := 0; i < 20; i++ {
        for idx, mk := range monkeys {
            if *debug {
                println("MONKEY ", idx)
            }
            for k, item := range mk.items {
                if *debug {
                    println(k, "OLD WORRY: ", item)
                }
                worry := mk.operation(item)
                worry = worry / 3
                // mk.items[k] = worry
                if *debug {
                    println(k, "NEW WORRY: ", worry)
                }

                monkeyDest := 0
                if mk.test(worry) {
                    monkeyDest = mk.monkeyTrue
                } else {
                    monkeyDest = mk.monkeyFalse
                }
                if *debug {
                    println(k, worry, "THROWING TO MK ", monkeyDest)
                }
                monkeys[monkeyDest].items = append(monkeys[monkeyDest].items, worry)
                mk.inspectCount++
            }
            mk.items = []int{}
        }
    }

    for i, mk := range monkeys {
        println("Monkey", i, "inspected items", mk.inspectCount, "times.")
    }

    // Answer: 54752
}

Expected result

% go run ./main.go
Monkey 0 inspected items 101 times.
Monkey 1 inspected items 95 times.
Monkey 2 inspected items 7 times.
Monkey 3 inspected items 105 times.

Got

% yaegi run ./main.go 
Monkey 0 inspected items 99 times.
Monkey 1 inspected items 97 times.
Monkey 2 inspected items 13 times.
Monkey 3 inspected items 108 times.

Yaegi Version

eee72d1aae664bf6627ef955215d886dc7b105c0

Additional Notes

I was solving the 11th advent of code: https://adventofcode.com/2022/day/11 and I noticed the result given by yaegi is different from the one with the go compiler.

I haven't had time to narrow it down yet, and to write a small repro. However, I strongly suspect the bug is connected to: monkeys[monkeyDest].items = append(monkeys[monkeyDest].items, worry) as the rest of the code is completely trivial.

I will update this issue with a better repro and title later.

mpl commented 1 year ago

Alright, here's the actual bug, which actually has nothing to do with what I was suspecting:

package main

import "strconv"

type monkey struct {
    test func() int
}

func main() {
    input := []string{"1", "2", "3"}

    var monkeys []*monkey

    //  for k, v := range input { // <-- no problem if we assign k to divisor, for example.
    for _, v := range input {
        kong := monkey{}

        // divisor := k <-- no problem
        divisor, err := strconv.Atoi(v)
        if err != nil {
            panic(err)
        }
        // divisor, _ := := strconv.Atoi(v) <-- also buggy
        println(divisor)
        kong.test = func() int {
            return divisor
        }
        monkeys = append(monkeys, &kong)
    }

    for _, mk := range monkeys {
        println(mk.test())
    }

}
% go run ./issues/1497/main.go
1
2
3
1
2
3
% 
% 
% 
% yaegi run ./issues/1497/main.go
1
2
3
3
3
3

So I believe it's the combination of the double return/assignment (from strconv.Atoi in that case), and the use in a closure, that leads to the bug. If the assignment to divisor does not come from a func with a double return, then there's no problem.