cue icon indicating copy to clipboard operation
cue copied to clipboard

evaluator: unexpected behavior of `list.Sum`

Open slewiskelly opened this issue 1 year ago • 4 comments
trafficstars

What version of CUE are you using (cue version)?

$ cue version
cue version v0.9.2

go version go1.22.4
      -buildmode exe
       -compiler gc
       -trimpath true
  DefaultGODEBUG httplaxcontentlength=1,httpmuxgo121=1,tls10server=1,tlsrsakex=1,tlsunsafeekm=1
     CGO_ENABLED 1
          GOARCH arm64
            GOOS darwin
cue.lang.version v0.9.2

Does this issue reproduce with the latest stable release?

Yes.

What did you do?

import "list"

receipt: {
        // Thanks for shopping at Big Kahuna Burger!

        cheeseburger: 3.00
        fries:        2.00
        sprite:       1.00

        total: list.Sum([ for _, v in receipt {v}])
}

What did you expect to see?

Would probably expect this to fail due to a cycle.

What did you see instead?

The value for total is 18, instead of 6.

$ cue export receipt.cue
{
    "receipt": {
        "cheeseburger": 3.00,
        "fries": 2.00,
        "sprite": 1.00,
        "total": 18
    }
}

slewiskelly avatar Jul 21 '24 13:07 slewiskelly

The following yields the correct result, though I would expect a conflict:

import "list"

receipt: {
        // Thanks for shopping at Big Kahuna Burger!

        cheeseburger: 3.00
        fries:        2.00
        sprite:       1.00

        total: list.Sum([for k, v in receipt {v}])
        total: list.Sum([for k, v in receipt if k != "total" {v}])
}

Finally, the following yields a cycle error, as expected:

import "list"

receipt: {
        // Thanks for shopping at Big Kahuna Burger.

        cheeseburger: 3.00
        fries:        2.00
        sprite:       1.00

        total: list.Sum([for k, v in receipt {v / 2}])
}

slewiskelly avatar Jul 21 '24 13:07 slewiskelly

Thanks very much for the report, @slewiskelly.

The new evaluator does see this as a cycle (the following test passes, as of f5b905cec6d6):

env CUE_EXPERIMENT=evalv3
! exec cue export x.cue
cmp stderr stderr.golden

-- x.cue --
import "list"

receipt: {
	// Thanks for shopping at Big Kahuna Burger!

	cheeseburger: 3.00
	fries:        2.00
	sprite:       1.00

	total: list.Sum([for _, v in receipt {v}])
}
-- stderr.golden --
receipt.3: structural cycle:
    ./x.cue:10:18

myitcv avatar Jul 22 '24 04:07 myitcv

Sorry, I completely forgot to try with CUE_EXPERIMENT=evalv3. As mentioned, this is identified as a cycle with that enabled:

$ CUE_EXPERIMENT=evalv3 cue export receipt.cue
receipt.3: structural cycle:
    ./receipt.cue:10:25

slewiskelly avatar Jul 22 '24 05:07 slewiskelly

@slewiskelly would you be happy with this issue being marked as resolved by the new evaluator, then? A number of issues around disjunctions, cycle detection, and performance are resolved thanks to the new evaluator's design, and our focus is on polishing up the new evaluator so that we may enable it by default for all users as soon as possible. Any time spent trying to backport any fixes to the old evaluator would slow down that transition, as you can imagine.

Just as with previous cases like https://github.com/cue-lang/cue/issues/3149, I will still add a test case to the repository with your example, to make sure that it keeps working with the new evaluator.

mvdan avatar Jul 23 '24 08:07 mvdan