evaluator: enable use of new evaluator via the Go API #3060
Assignees
Labels
Comments
|
Spoke briefly with @mpvl about this today. This task is not as simple as I thought, as it is more than just threading an option through between the cuecontext and internal evaluator packages. A number of the public APIs that use internal APIs, such as in the We still intend to get to this soon. With Marcel we agreed that he would get to this after finishing the current batch of fixes already in progress, as he's working on bits of code relating to structure sharing. |
cueckoo
pushed a commit
that referenced
this issue
Apr 26, 2024
This adds a few more tests that escaped the mechanical adjustments of the previous CL to using the matrix. It also adds TODO* functions to exclude tests from certain test combinations if they currently fail. This clarifies which tests are not yet compatible with the new evaluator. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I03e8b6811b012070d0379f7400599d2c07c2ae64
cueckoo
pushed a commit
that referenced
this issue
Apr 26, 2024
Displaying conjunct groups are usuful in some test outputs, but not really in error messages. It seems reasonable to only print them in non-compact mode. This also fixes a test breakages in the API. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Ic9ae9a31f4067349b7dcc7baf75e60552b5bb617
cueckoo
pushed a commit
that referenced
this issue
Apr 26, 2024
Defaults are constructed from final values, so we should ensure that the value is fully dereferenced before getting a default. This fixes most tests of TestFields. We added a per-test todo field to exclude the rest. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iedfcbb679d74f8be39a6ce1ed6109577c969c6c2
cueckoo
pushed a commit
that referenced
this issue
Apr 26, 2024
Now we have structure sharing, dereferencing values causes nodes to lose information. Before, indirection was done to deal with a status that was not properly updated. This is now also fixed in this CL and it is now safe to remove this call to Indirect. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Id916b0738bd209177860f46e7d1e6f75d5b15a56
cueckoo
pushed a commit
that referenced
this issue
Apr 26, 2024
This adds a few more tests that escaped the mechanical adjustments of the previous CL to using the matrix. It also adds TODO* functions to exclude tests from certain test combinations if they currently fail. This clarifies which tests are not yet compatible with the new evaluator. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I03e8b6811b012070d0379f7400599d2c07c2ae64
cueckoo
pushed a commit
that referenced
this issue
Apr 26, 2024
Displaying conjunct groups are usuful in some test outputs, but not really in error messages. It seems reasonable to only print them in non-compact mode. This also fixes a test breakages in the API. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Ic9ae9a31f4067349b7dcc7baf75e60552b5bb617
cueckoo
pushed a commit
that referenced
this issue
Apr 26, 2024
Defaults are constructed from final values, so we should ensure that the value is fully dereferenced before getting a default. This fixes most tests of TestFields. We added a per-test todo field to exclude the rest. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iedfcbb679d74f8be39a6ce1ed6109577c969c6c2
cueckoo
pushed a commit
that referenced
this issue
Apr 26, 2024
Now we have structure sharing, dereferencing values causes nodes to lose information. Before, indirection was done to deal with a status that was not properly updated. This is now also fixed in this CL and it is now safe to remove this call to Indirect. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Id916b0738bd209177860f46e7d1e6f75d5b15a56
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
This adds a few more tests that escaped the mechanical adjustments of the previous CL to using the matrix. It also adds TODO* functions to exclude tests from certain test combinations if they currently fail. This clarifies which tests are not yet compatible with the new evaluator. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I03e8b6811b012070d0379f7400599d2c07c2ae64
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
Displaying conjunct groups are usuful in some test outputs, but not really in error messages. It seems reasonable to only print them in non-compact mode. This also fixes a test breakages in the API. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Ic9ae9a31f4067349b7dcc7baf75e60552b5bb617
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
Defaults are constructed from final values, so we should ensure that the value is fully dereferenced before getting a default. This fixes most tests of TestFields. We added a per-test todo field to exclude the rest. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iedfcbb679d74f8be39a6ce1ed6109577c969c6c2
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
Now we have structure sharing, dereferencing values causes nodes to lose information. Before, indirection was done to deal with a status that was not properly updated. This is now also fixed in this CL and it is now safe to remove this call to Indirect. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Id916b0738bd209177860f46e7d1e6f75d5b15a56
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
The new evaluator did not erase the Conjuncts field for disjunctions. We plan to record the actual conjuncts that went in to a disjunct for the new evaluator, but this has not been implemented yet. The API depended on the Conjuncts to be nil for disjuncts. We mimic this old behavior, for now, until we have the above implemented. Also, the old evaluator would filter default values from the original expression. This did not handle the ConjunctGroup case yet. This CL implements that. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I1e8e653ede94f54df6f9e99f50b2931ef3e3674f
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
The new evaluator does not implement IsSpan. Instead it kept track of closedness through the new closeContext construct. The relevant information on whether or not a struct is closed is now copied to fields in the Vertex. Rather than just having Closed, it keeps trackof this with an additional field HasEllipsis. This allows accumulating information as it comes in, instead of possibly revert setting Closed, which is less error prone. This is especially so because the old evaluator uses Closed in a slightly different way. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I90672c3a37fd441c0a718b6349961e33df2026d0
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
This adds a few more tests that escaped the mechanical adjustments of the previous CL to using the matrix. It also adds TODO* functions to exclude tests from certain test combinations if they currently fail. This clarifies which tests are not yet compatible with the new evaluator. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I03e8b6811b012070d0379f7400599d2c07c2ae64 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1193956 Reviewed-by: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
Displaying conjunct groups are usuful in some test outputs, but not really in error messages. It seems reasonable to only print them in non-compact mode. This also fixes a test breakages in the API. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Ic9ae9a31f4067349b7dcc7baf75e60552b5bb617 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1193957 Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
matchPattern did not handle the case where Feature f was `AnyString`. It now does. This also means that more places in the code should check for label != nil, as it is now possible for it to be nil even if kind is string. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I3a814b92b0d28dd8408a95beff329042938f7192
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
Defaults are constructed from final values, so we should ensure that the value is fully dereferenced before getting a default. This fixes most tests of TestFields. We added a per-test todo field to exclude the rest. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iedfcbb679d74f8be39a6ce1ed6109577c969c6c2 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1193958 TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
Now we have structure sharing, dereferencing values causes nodes to lose information. Before, indirection was done to deal with a status that was not properly updated. This is now also fixed in this CL and it is now safe to remove this call to Indirect. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Id916b0738bd209177860f46e7d1e6f75d5b15a56 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1193959 TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
The new evaluator did not erase the Conjuncts field for disjunctions. We plan to record the actual conjuncts that went in to a disjunct for the new evaluator, but this has not been implemented yet. The API depended on the Conjuncts to be nil for disjuncts. We mimic this old behavior, for now, until we have the above implemented. Also, the old evaluator would filter default values from the original expression. This did not handle the ConjunctGroup case yet. This CL implements that. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I1e8e653ede94f54df6f9e99f50b2931ef3e3674f Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194048 Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
The new evaluator does not implement IsSpan. Instead it kept track of closedness through the new closeContext construct. The relevant information on whether or not a struct is closed is now copied to fields in the Vertex. Rather than just having Closed, it keeps trackof this with an additional field HasEllipsis. This allows accumulating information as it comes in, instead of possibly revert setting Closed, which is less error prone. This is especially so because the old evaluator uses Closed in a slightly different way. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I90672c3a37fd441c0a718b6349961e33df2026d0 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194049 Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
matchPattern did not handle the case where Feature f was `AnyString`. It now does. This also means that more places in the code should check for label != nil, as it is now possible for it to be nil even if kind is string. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I3a814b92b0d28dd8408a95beff329042938f7192 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194051 TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
The new evalautor reports different, but correct, positions, that are actually better than those of the old evaluator. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I56d062eec11d22f5eac1fe8751071cd9f0fbb9a9
cueckoo
pushed a commit
that referenced
this issue
Apr 29, 2024
For certain cases in the API the DynamicLabel needs to be explicitly set. Also, Template needs to recognize when Lookup can applied for the new evaluator. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iad2a9ff913588ec3151ca61121aa714aa0b571f6
cueckoo
pushed a commit
that referenced
this issue
Apr 30, 2024
The new evaluator computes closedness quite differently from the old one. This CL adapts the API to use the new data. Note that the data the new evaluator generates is considerably more convenient. That is why the logic is quite a bit simpler. The logic could probably be simplified more. Also note that HasEllipsis seems to be not computed correclty entirely. Althoug it seems that many tests improve if we add the check to isClosed or IsClosedStruct, there are some cases where this causes errors. This seems to be the case with one of the tests in TestAllows. As this only manifests itself when structure sharing is disabled, we just mark this test as todo and leave the investigation for later. Issue #3060 Issue #543 Issue #2884 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I98e8c25e0680674e2aaa9f743a49b39920d7b266
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
Passing the two remaining tests required rewriting the vertices method to make use of the new data structure for optional fields and pattern constraints. The resulting method could ditch the use of vertexFeatures and also is more accurate. Some tests have been added to demonstrate this. A bug in adt.Equal was fixed to make these tests pass, where Top was not properly equated. As all fields now pass, todo_v3 was removed. We added skip_v2 to skip tests of the old evaluator. We used skip instead of todo here, because we do not intend to fix this tests for v2. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I2f1730249ba75080e395e304704b9308fcfe28d6
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
Lookup dereferences all vertex indirections, including those for structure-sharing. If indirection happens too early it may lead to incorrect arc types. This changes postpones the dereference. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iae18d29d4d32b2acc762df68d63f46e99e36da56
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
We should enable this in case field properties are not handled correctly for structure sharing. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I85f70df74b3c4449ee4f5c729668638b8246882f
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
This changes Runtime.SetSettings back to SetVersion and adds AddDebugOptions. SetSettings has now served its purpose of identifying all call sites where these options need to be set. :) Note that we enable the API before it is fully working. But it probably works well enough for many applications. For open options search for the following across the repo: - '-- diff(/.*)?todo/p? --' in txtar files - todo_* fields in table-driven tests - TODO_(V3|Sharing|NoSharing) function calls in table-driven tests - TODO(evalv3) around matrix tests (like in trim_test.go) Removed use of "stderr 'str'" in txtar, as it does not support CUE_UPDATE=1. Closes #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Ib3970d867363711f461c8c303abfdb0402706fff
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
And change calls accordingly. We can now use the cuetdtest package. Doing so limits us from making that package depend on cue and cuecontext, but that was already the case. It only depends on internal/core/runtime. Using a single package for matrix tests does not only simplify code and make things less confusing, but also makes it easier to locate TODOs related to matrix tests, as there will be less functions and one less packge to check. Added new newRuntime wrapper for creating a cue.Runtime. Changed signature of getValue. Other than that this is a straightforward change. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I659c83d3aa0c86b333ccf68f40f1e85497207c1a
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
Passing the two remaining tests required rewriting the vertices method to make use of the new data structure for optional fields and pattern constraints. The resulting method could ditch the use of vertexFeatures and also is more accurate. Some tests have been added to demonstrate this. A bug in adt.Equal was fixed to make these tests pass, where Top was not properly equated. As all fields now pass, todo_v3 was removed. We added skip_v2 to skip tests of the old evaluator. We used skip instead of todo here, because we do not intend to fix this tests for v2. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I2f1730249ba75080e395e304704b9308fcfe28d6
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
Lookup dereferences all vertex indirections, including those for structure-sharing. If indirection happens too early it may lead to incorrect arc types. This changes postpones the dereference. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iae18d29d4d32b2acc762df68d63f46e99e36da56
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
We should enable this in case field properties are not handled correctly for structure sharing. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I85f70df74b3c4449ee4f5c729668638b8246882f
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
And change calls accordingly. We can now use the cuetdtest package. Doing so limits us from making that package depend on cue and cuecontext, but that was already the case. It only depends on internal/core/runtime. Using a single package for matrix tests does not only simplify code and make things less confusing, but also makes it easier to locate TODOs related to matrix tests, as there will be less functions and one less packge to check. Added new newRuntime wrapper for creating a cue.Runtime. Changed signature of getValue. Other than that this is a straightforward change. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I659c83d3aa0c86b333ccf68f40f1e85497207c1a Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194240 Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
Passing the two remaining tests required rewriting the vertices method to make use of the new data structure for optional fields and pattern constraints. The resulting method could ditch the use of vertexFeatures and also is more accurate. Some tests have been added to demonstrate this. A bug in adt.Equal was fixed to make these tests pass, where Top was not properly equated. As all fields now pass, todo_v3 was removed. We added skip_v2 to skip tests of the old evaluator. We used skip instead of todo here, because we do not intend to fix these tests for v2. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I2f1730249ba75080e395e304704b9308fcfe28d6
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
Lookup dereferences all vertex indirections, including those for structure-sharing. If indirection happens too early it may lead to incorrect arc types. This changes postpones the dereference. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iae18d29d4d32b2acc762df68d63f46e99e36da56
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
We should enable this in case field properties are not handled correctly for structure sharing. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I85f70df74b3c4449ee4f5c729668638b8246882f
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
Passing the two remaining tests required rewriting the vertices method to make use of the new data structure for optional fields and pattern constraints. The resulting method could ditch the use of vertexFeatures and also is more accurate. Some tests have been added to demonstrate this. A bug in adt.Equal was fixed to make these tests pass, where Top was not properly equated. As all fields now pass, todo_v3 was removed. We added skip_v2 to skip tests of the old evaluator. We used skip instead of todo here, because we do not intend to fix these tests for v2. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I2f1730249ba75080e395e304704b9308fcfe28d6 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194251 TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
Lookup dereferences all vertex indirections, including those for structure-sharing. If indirection happens too early it may lead to incorrect arc types. This changes postpones the dereference. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iae18d29d4d32b2acc762df68d63f46e99e36da56 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194402 Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
cueckoo
pushed a commit
that referenced
this issue
May 7, 2024
We should enable this in case field properties are not handled correctly for structure sharing. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I85f70df74b3c4449ee4f5c729668638b8246882f Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194404 Reviewed-by: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
cueckoo
pushed a commit
that referenced
this issue
May 8, 2024
Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I755e74f9197ab9d2643f40765d32c94754d0eed2
cueckoo
pushed a commit
that referenced
this issue
May 8, 2024
Resolve now always derefences Vertex values that do not contribute valuable information. This is also the right granularity needed for the old evaluator, so we make the change for both, simplifying the code. We also remove the indirection in dep, which is now no longer needed. We could have merged this CL with the change in dep, but we keep it as a separate CL so that if this change causes issues, it can be safely rolled back without compromising the fix in dep. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iba414b9d1db6479274fe5b80574ae9704e4b0f6f
cueckoo
pushed a commit
that referenced
this issue
May 8, 2024
With the implementation of the new evaluator, especially with structure sharing, it is often no longer correct to dereference a Vertex unconditionally. So the new evaluator removed some dereferences, sometimes a bit too aggressively. This changes patches one of these changes locally. In a followup CL we provide a more general fix and remove this change again. This allows us to fall back on this change if that results in any issues. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I755e74f9197ab9d2643f40765d32c94754d0eed2
cueckoo
pushed a commit
that referenced
this issue
May 8, 2024
Resolve now always derefences Vertex values that do not contribute valuable information. This is also the right granularity needed for the old evaluator, so we make the change for both, simplifying the code. We also remove the indirection in dep, which is now no longer needed. We could have merged this CL with the change in dep, but we keep it as a separate CL so that if this change causes issues, it can be safely rolled back without compromising the fix in dep. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iba414b9d1db6479274fe5b80574ae9704e4b0f6f
cueckoo
pushed a commit
that referenced
this issue
May 8, 2024
With the implementation of the new evaluator, especially with structure sharing, it is often no longer correct to dereference a Vertex unconditionally. So the new evaluator removed some dereferences, sometimes a bit too aggressively. This changes patches one of these changes locally. In a followup CL we provide a more general fix and remove this change again. This allows us to fall back on this change if that results in any issues. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I755e74f9197ab9d2643f40765d32c94754d0eed2
cueckoo
pushed a commit
that referenced
this issue
May 8, 2024
Resolve now always derefences Vertex values that do not contribute valuable information. This is also the right granularity needed for the old evaluator, so we make the change for both, simplifying the code. We also remove the indirection in dep, which is now no longer needed. We could have merged this CL with the change in dep, but we keep it as a separate CL so that if this change causes issues, it can be safely rolled back without compromising the fix in dep. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iba414b9d1db6479274fe5b80574ae9704e4b0f6f
cueckoo
pushed a commit
that referenced
this issue
May 8, 2024
With the implementation of the new evaluator, especially with structure sharing, it is often no longer correct to dereference a Vertex unconditionally. So the new evaluator removed some dereferences, sometimes a bit too aggressively. This changes patches one of these changes locally. In a followup CL we provide a more general fix and remove this change again. This allows us to fall back on this change if that results in any issues. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I755e74f9197ab9d2643f40765d32c94754d0eed2 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194445 Reviewed-by: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
cueckoo
pushed a commit
that referenced
this issue
May 8, 2024
Resolve now always derefences Vertex values that do not contribute valuable information. This is also the right granularity needed for the old evaluator, so we make the change for both, simplifying the code. We also remove the indirection in dep, which is now no longer needed. We could have merged this CL with the change in dep, but we keep it as a separate CL so that if this change causes issues, it can be safely rolled back without compromising the fix in dep. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iba414b9d1db6479274fe5b80574ae9704e4b0f6f Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194446 TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
cueckoo
pushed a commit
that referenced
this issue
May 8, 2024
Fixes in dep have fixed these tests. See https://review.gerrithub.io/c/cue-lang/cue/+/1194445 Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Ifee32c22b69172fbad51fd53c1f0c2996703bb47
cueckoo
pushed a commit
that referenced
this issue
May 13, 2024
Fixes in dep have fixed these tests. See https://review.gerrithub.io/c/cue-lang/cue/+/1194445 Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Ifee32c22b69172fbad51fd53c1f0c2996703bb47
cueckoo
pushed a commit
that referenced
this issue
May 13, 2024
The use of the Vertex.Structs fields has been almost completely eliminated in the new evaluator. It is, however, still used for some things, like keeping track of attributes. This change fixes the bookkeeping of StructLits. It changes the signature of Init to allow it to determine if a full init is necessary. This also gives us some compiler support to check if all call sites in the old evaluator have an equivalent in the new one. Note that one use of Init was missing in the new evaluator. The case used in list processing seems unnecessary in the new evaluator. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I2eb3319f0f3ad26c3d64adcff819016a24ec6e7d
cueckoo
pushed a commit
that referenced
this issue
May 13, 2024
Fixes in dep have fixed these tests. See https://review.gerrithub.io/c/cue-lang/cue/+/1194445 Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Ifee32c22b69172fbad51fd53c1f0c2996703bb47 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194464 Reviewed-by: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
cueckoo
pushed a commit
that referenced
this issue
May 13, 2024
The use of the Vertex.Structs fields has been almost completely eliminated in the new evaluator. It is, however, still used for some things, like keeping track of attributes. This change fixes the bookkeeping of StructLits. It changes the signature of Init to allow it to determine if a full init is necessary. This also gives us some compiler support to check if all call sites in the old evaluator have an equivalent in the new one. Note that one use of Init was missing in the new evaluator. The case used in list processing seems unnecessary in the new evaluator. Issue #3060 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: I2eb3319f0f3ad26c3d64adcff819016a24ec6e7d Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194658 TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is a placeholder issue for providing a means of enabling the new evaluator via the Go API.
It is a follow-up to #2884, as well as #2886, which already exposes the experiment to cmd/cue via $CUE_EXPERIMENT.
The new evaluator is not be enabled by default: an uncontrolled flip from one evaluator to another would be an incredibly risky and unnecessary move. Just like some users are already enabling the experiment via e.g.
CUE_EXPERIMENT=evalv3 cue export, we should expose a way for Go API users to opt into it as well.My intuition is that we would add something to pass to https://pkg.go.dev/cuelang.org/go/cue/cuecontext#New, for example a
cuecontext.Optionto select a particular evaluator version, so that we can also reuse it for future versions we might need. When the option isn't given, we would continue with the current default.The text was updated successfully, but these errors were encountered: