cue icon indicating copy to clipboard operation
cue copied to clipboard

evalv3: strange behaviour with chaining of list comprehensions

Open myitcv opened this issue 1 year ago • 3 comments

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

$ cue version
cue version v0.0.0-20240507175810-9efad9041981

go version go1.22.1
      -buildmode exe
       -compiler gc
  DefaultGODEBUG httplaxcontentlength=1,httpmuxgo121=1,tls10server=1,tlsrsakex=1,tlsunsafeekm=1
     CGO_ENABLED 1
          GOARCH arm64
            GOOS linux
             vcs git
    vcs.revision 9efad90419811c9a27682857ffdc9324cdb7e094
        vcs.time 2024-05-07T17:58:10Z
    vcs.modified false
cue.lang.version v0.9.0

Does this issue reproduce with the latest release?

Yes

What did you do?

# old evaluator
env CUE_EXPERIMENT=''
exec cue export x.cue
cmp stdout stdout.golden

# new evaluator
env CUE_EXPERIMENT='evalv3'
exec cue export x.cue
cmp stdout stdout.golden

-- x.cue --
import "math/bits"

x: [...bool]
x: [true, false, true, true]
y: [for _, v in x {[if v {1}, 0][0]}]
z: [for i, v in y {bits.Set(*z[i-1] | 0, len(y)-i-1, v)}][3]

-- stdout.golden --
{
    "x": [
        true,
        false,
        true,
        true
    ],
    "y": [
        1,
        0,
        1,
        1
    ],
    "z": 1
}

What did you expect to see?

Passing test.

What did you see instead?

# old evaluator (0.012s)
# new evaluator (0.011s)
> env CUE_EXPERIMENT='evalv3'
> exec cue export x.cue
[stderr]
z: error in call to math/bits.Set: non-concrete value _:
    ./x.cue:6:20
[exit status 1]
FAIL: /tmp/testscript820382168/repro.txtar/script.txtar:8: unexpected command failure

Note that it is understood that the reference in the declaration of z to *z[i-1] | 0 is "wrong" by virtue of the result of z being a scalar value (contrast https://cuelang.org/play/?id=MSdriK7Ernf#w=function&i=cue&f=eval&o=cue) but reporting this actual error for completeness' sake.

myitcv avatar May 08 '24 05:05 myitcv

Simplification 1:

import "math/bits"
z: [0, bits.Set(*z[0] | 0, 0, 0)][1] // Same error

Simplification 2:

z: [0, z[0]][1] // results in `_`

mpvl avatar Aug 12 '24 17:08 mpvl

BTW, it is correct for this to be an error. The error message is just confusing.

The old evaluator was incorrect here: z is an integer value in the final result, so z[x] is not a valid expression.

It remains a bit open what a correct error would be, but probably a cycle error is the most appropriate.

mpvl avatar Aug 13 '24 09:08 mpvl

The old evaluator was incorrect here: z is an integer value in the final result, so z[x] is not a valid expression.

Yep indeed, hence why I wrote in the original description:

Note that it is understood that the reference in the declaration of z to *z[i-1] | 0 is "wrong" by virtue of the result of z being a scalar value (contrast https://cuelang.org/play/?id=MSdriK7Ernf#w=function&i=cue&f=eval&o=cue) but reporting this actual error for completeness' sake.

Regarding:

It remains a bit open what a correct error would be, but probably a cycle error is the most appropriate.

Seems to make sense. That would give a "hint" that the code refers to self, when it really shouldn't.

myitcv avatar Aug 19 '24 16:08 myitcv

The old evaluator was incorrect here: z is an integer value in the final result, so z[x] is not a valid expression.

I've just looked at this again, and I'm not sure I can follow why we see an error. Because the expression *z[i-1] | 0 should presumably eliminate the invalid element from the disjunction, rather than erroring? i.e. in case z is an integer, then *z[i-1] | 0 should evaluate to 0.

(which invalidates the original expectation, but we can fix that assuming we agree on what the behaviour should be with respect to the disjunction)

myitcv avatar Feb 15 '25 14:02 myitcv

It remains a bit open what a correct error would be, but probably a cycle error is the most appropriate.

@mpvl - just to confirm, is such an error "serious enough" that the evaluator fails early, rather than simply dropping that element from the disjunction?

myitcv avatar Feb 15 '25 14:02 myitcv

That is an interesting question. One could argue that this is a clear bug and not intended. So I would say it is preferable fail here.

That said, this works

z: 2
x: *z[3] | 0

which would indeed argue in favor of dropping the value from the disjunction.

mpvl avatar Feb 15 '25 14:02 mpvl

That said, this works

That's exactly the kind of construct I had to reason that this shouldn't be a failure, indeed.

One could argue that this is a clear bug and not intended. So I would say it is preferable fail here.

I'd tend to agree, but in the past we've tended to err on the side of "these sorts of things should be reported by vet".

I'll leave this open: it's also not clear to me the evalv3-win label is appropriate, because of the inconsistency you point out in https://github.com/cue-lang/cue/issues/3124#issuecomment-2660955720

myitcv avatar Feb 15 '25 14:02 myitcv