fix: equal() detects functions in nested containers during equality comparison#959
Closed
He-Pin wants to merge 1 commit into
Closed
fix: equal() detects functions in nested containers during equality comparison#959He-Pin wants to merge 1 commit into
He-Pin wants to merge 1 commit into
Conversation
…omparison
Motivation:
The `equal()` function in Evaluator only rejected function comparison at
the top level (`(function() 3) == (function() 3)`) but not when functions
appeared inside arrays or objects. This diverges from go-jsonnet and
jsonnet-cpp which both raise "cannot test equality of functions" when
functions are encountered at any nesting depth.
Modification:
- Array comparison loop: detect Val.Func in either element before
recursing, including the shared-thunk fast path (xe eq ye)
- Object comparison loop: detect Val.Func in field values before
recursing via equal()
- Top-level catch-all: reject when both sides are Val.Func
Added golden tests covering functions in arrays, objects, and mixed
function-vs-non-function container comparisons.
Result:
`[function() 3] == [function() 4]` and `{a: function() 3} == {a: function() 4}`
now correctly raise "cannot test equality of functions", aligning with
go-jsonnet and jsonnet-cpp behavior.
References:
- go-jsonnet: builtins.go equal() checks for function type in all paths
- jsonnet-cpp: vm.cpp equality check rejects function operands
6 tasks
Contributor
Author
|
Closing: PR #958 already handles all nested function equality cases through the top-level The explicit checks in array and object comparison loops added by this PR are redundant — the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
equal()inEvaluator.scalaonly rejected function comparison at the top level but missed functions nested inside arrays or objects. All three reference implementations (go-jsonnet, jsonnet-cpp, jrsonnet) agree — when two functions are compared inside any container (array/object), at any nesting depth, the result is an error. When a function is compared against a non-function, the result isfalse(no error). sjsonnet was incorrectly returningfalsefor function-vs-function in containers.Modification
Val.Funcin either element before recursing, including the shared-thunk fast path (xe eq ye) where forcing a shared lazy thunk may yield a functionVal.Funcin field values before recursing viaequal()Val.FuncResult
[function() 3] == [function() 4]and{a: function() 3} == {a: function() 4}now correctly raise "cannot test equality of functions", aligning with all three reference implementations.f == g[f] == [g]false(bug){a: f} == {a: g}false(bug)[[f]] == [[g]]false(bug)[f] == [1]falsefalsefalse{a: f} == {a: 1}falsefalsefalseTest plan
error.equality_function_in_array.jsonnet—[f] == [g]raises errorerror.equality_function_in_object.jsonnet—{a: f} == {a: g}raises errorequality_function_mixed_container.jsonnet— function vs non-function returns false (no error)mill sjsonnet.jvm.3_3_7.test)References
builtins.goequal()checks for function type in all comparison pathsvm.cppequality check rejects function operands at any depth