-
Notifications
You must be signed in to change notification settings - Fork 294
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: Regression: result.#_list.0: #in.c1 undefined as #in is incomplete (type _): #2113
Comments
Thanks for the report, @bttk. Investigating now. |
I've reduced this down to the follow. What did you do?
What did you expect to see?A passing test. This passes with v0.4.3 What did you see instead?
Bisected to 748a685 Marking as v0.5.0. |
Simpler reproducer:
|
Issue #2113 Issue #2131 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I94cf1a28c3261f13dfef6b7214b59baed1ef0a62 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/546597 Unity-Result: CUEcueckoo <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Roger Peppe <[email protected]>
There were some cases where conjuncts could be added after the Conjuncts state had been reached. This is okay, as there is a separate mechanism to verify such additions are legal. But not adding those can lead to lost information. Tests for this were added before and will be fixed after removing a workaround that worked around the issue fixed here, but introduced errors itself. Issue #2113 Fixes #2131 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I74522731007393b96c6ba69acc688ca044067f3f Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/546598 Reviewed-by: Roger Peppe <[email protected]> Unity-Result: CUEcueckoo <[email protected]> TryBot-Result: CUEcueckoo <[email protected]>
With the merging of handing the Partial and Conjuncts state, this special casing is no longer necessary. As the special casing is actually incorrect, this fixes a couple of bugs. This fixes the original issue reported in 2113. We leave it open, though, as there is still a related issue outstanding. Issue #2113 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I365658a63528ad340d0b4506fc4a7b4bd1566e33 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/546611 Reviewed-by: Roger Peppe <[email protected]> TryBot-Result: CUEcueckoo <[email protected]>
A different reproducer: #Func: {
#in2: _
_
}
#fn: #Func
#fn: {
#in2: _
#in2.c1
}
x: #fn & {#in2: {c1: "V 1"}, _} Again, the constraint Per @mpvl this (also) appears to be an ordering thing. |
Issue #2113 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: Ie30cdbb2bd9e6444de646b6b3ea8369ce4aee060 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/546856 Reviewed-by: Paul Jolly <[email protected]> Unity-Result: CUEcueckoo <[email protected]> TryBot-Result: CUEcueckoo <[email protected]>
This prevents non-monotonic checks to be evaluated too early. Issue #2113 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I4e63d67152452b5df5184de65e0a679251c83826 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/546858 Unity-Result: CUEcueckoo <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Paul Jolly <[email protected]>
This improves one error message. It's main purpose is to ultimately pass the state to various evaluation functions. It takes a somewhat conservative stance to not pass the state for calls where this is not necessary. For instance, the Partial mode is typically used to get a scalar value as further evaluation is unnecessary. In fact, elevating the evaluation may thwart the ability to resolve cycles in that case. Issue #2113 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I504aaacc976e9f1a07460c680456fc45004b575f Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/546857 Reviewed-by: Paul Jolly <[email protected]> Unity-Result: CUEcueckoo <[email protected]> TryBot-Result: CUEcueckoo <[email protected]>
Issue #2113 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I3f868e3c8a9ac88d79d70aa60110318ad18e1c5a Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/546859 Reviewed-by: Paul Jolly <[email protected]> Unity-Result: CUEcueckoo <[email protected]> TryBot-Result: CUEcueckoo <[email protected]>
Before this CL it may be set too early. Some of the tests that are added are still broken, and will be fixed in subsequent CLs. Issue #2113 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I763a4046ad11d7de407b6be188f3d3bbbe2669d3 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/546860 Reviewed-by: Paul Jolly <[email protected]> Unity-Result: CUEcueckoo <[email protected]> TryBot-Result: CUEcueckoo <[email protected]>
See comments in internal/core/adt/context.go. This fixes broken tests in scalars/embed.txtar Issue #2113 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I32c1a34bc91e3df7d7daf7da209e89204eaba19d Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/546861 Reviewed-by: Paul Jolly <[email protected]> Unity-Result: CUEcueckoo <[email protected]> TryBot-Result: CUEcueckoo <[email protected]>
looks like it's fixed in
|
Yes indeed, @bttk! You can see above the list of fixes that went into this one. Thanks again for raising, I will mark this as closed now. |
What version of CUE are you using (
cue version
)?Does this issue reproduce with the latest release?
Yes
What did you do?
What did you expect to see?
$ go run cuelang.org/go/cmd/[email protected] export --all-errors --verbose '--out=text' --inject 'table=table_1_csv_import' --inject 'func=func_1' --inject 'mapper=text' --inject 'join=|' --inject 'outTable=table_1_csv_import' --expression result input.cue tools/cue/test/testdata/func_1.cue tools/cue/mapper/apply.cue
What did you see instead?
$ go run cuelang.org/go/cmd/[email protected] export --all-errors --verbose '--out=text' --inject 'table=table_1_csv_import' --inject 'func=func_1' --inject 'mapper=text' --inject 'join=|' --inject 'outTable=table_1_csv_import' --expression result input.cue tools/cue/test/testdata/func_1.cue tools/cue/mapper/apply.cue
The text was updated successfully, but these errors were encountered: