Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

evaluator: phase out and deprecate arithmetic on lists #2237

Closed
mvdan opened this issue Feb 1, 2023 · 4 comments
Closed

evaluator: phase out and deprecate arithmetic on lists #2237

mvdan opened this issue Feb 1, 2023 · 4 comments
Assignees
Labels
NeedsFix spec-deviation Bugs where the implementation does not follow the spec.

Comments

@mvdan
Copy link
Member

mvdan commented Feb 1, 2023

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

$ cue version
cue version v0.5.0-beta.3

      -buildmode exe
       -compiler gc
        -ldflags -w -s
     CGO_ENABLED 1
          GOARCH amd64
            GOOS linux
         GOAMD64 v3

Does this issue reproduce with the latest release?

Yes.

What did you do?

! exec cue export root.cue

-- root.cue --
import "list"

foo: [1, 2] + [3]
bar: list.Concat([[1, 2], [3]])

What did you expect to see?

testscript -v repro-cmd.txtar should succeed; arithmetic on lists hasn't been part of the spec since 2021, per https://cue-review.googlesource.com/c/cue/+/8063. Right now, that feature continues to work, and I didn't even notice that I shouldn't be using it until @myitcv reminded me.

I expect cue export to fail on arithmetic on lists, or at least for a command like cue fmt to rewrite + with list.Concat as a transition step. For example, we could make CUE 0.6 apply this cue fmt rewrite, and a future version (0.8? 0.9?) reject the old form entirely.

What did you see instead?

An old and removed spec feature continues to work without hiccups.

@mvdan mvdan added the NeedsFix label Feb 1, 2023
@myitcv myitcv added this to the v0.6.0 required fields milestone Feb 1, 2023
@myitcv
Copy link
Member

myitcv commented Feb 1, 2023

Marking this a v0.6.0 pending discussion with @mpvl

@myitcv
Copy link
Member

myitcv commented Jun 14, 2023

Moving forward to v0.6.x to implement a "fix" to rewrite to use list.Concat. Later deprecation etc to follow in a later release.

@mvdan mvdan added the spec-deviation Bugs where the implementation does not follow the spec. label Jul 31, 2023
@4ad
Copy link
Contributor

4ad commented Jun 20, 2024

This should also remove multiplication on lists.

@mvdan mvdan self-assigned this Aug 10, 2024
@mvdan mvdan removed this from the v0.6.x milestone Aug 10, 2024
@cuematthew cuematthew self-assigned this Aug 29, 2024
@mvdan
Copy link
Member Author

mvdan commented Aug 29, 2024

We closed this prematurely; the only merged commit does the cue mod fix step, but the step to actually remove the feature is https://review.gerrithub.io/c/cue-lang/cue/+/1200221, yet to be merged.

@mvdan mvdan reopened this Aug 29, 2024
@cueckoo cueckoo closed this as completed in 46fb300 Sep 3, 2024
cueckoo pushed a commit that referenced this issue Jan 28, 2025
The existing conversion used the old multiplication of lists when
converting Go Array types to CUE. E.g. [3]string{} -> [string] * 3.
This was deprecated years ago, and removed in issue #2237 and
https://review.gerrithub.io/c/cue-lang/cue/+/1200221

Instead, we now convert [3]string{} -> list.Repeat([string], 3)

Signed-off-by: Matthew Sackman <[email protected]>
Change-Id: I49aa552a5d7cb04ba5279b10d54d6b4804747f0e
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1207907
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix spec-deviation Bugs where the implementation does not follow the spec.
Projects
Status: v0.11.0-alpha.1
Development

No branches or pull requests

5 participants