Skip to content

fix: apply addition semantics to object extension - #1120

Open
He-Pin wants to merge 1 commit into
databricks:masterfrom
He-Pin:fix/objextend-nonobject-lhs
Open

fix: apply addition semantics to object extension#1120
He-Pin wants to merge 1 commit into
databricks:masterfrom
He-Pin:fix/objextend-nonobject-lhs

Conversation

@He-Pin

@He-Pin He-Pin commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Motivation

Jsonnet defines lhs { body } as syntactic sugar for lhs + { body }. Previously, sjsonnet required an object on the left and rejected valid expressions such as "hello" { x: 1 } with Expected object, found string. This expression should return "hello{\"x\": 1}", just like explicit addition.

Modification

  • Preserve the existing object-inheritance fast path, including self, super, assertions, and lazy fields.
  • For a non-object left operand, delegate to the existing BinaryOp + evaluator using the already evaluated value. This evaluates the original left expression once and shares object construction, stringification, and error reporting with explicit addition.
  • Add regression coverage for all left-operand types, static and dynamic object bodies, comprehensions, hidden fields, evaluation order, lazy errors, inheritance, shared object-body caches, and the std.foldl fallback. Include executable number/array error goldens and 14 success assertions.

Result

String/object extension now follows normal addition semantics. Unsupported left-operand types still fail, with the same diagnostics as +. The existing strict-mode restriction on consecutive object bodies is preserved.

Validation completed locally:

  • JVM Scala 2.12.21, 2.13.18, and 3.3.8: 677/677 tests passed per version (2,031 total).
  • Formatting checks passed for all JVM Scala versions; the Scala 3.3.8 assembly built successfully.
  • The assembled CLI matched the success, number-error, and array-error golden files, including exit codes and diagnostics.
  • The 14 success assertions passed on Go Jsonnet 0.21.0 and 0.22.0.
./mill --no-daemon -j 1 'sjsonnet.jvm[_].test' + 'sjsonnet.jvm[_].checkFormat' + 'sjsonnet.jvm[3.3.8].assembly'

JS, WASM, Native, and JMH were not run locally.

References

@He-Pin
He-Pin marked this pull request as draft September 3, 2026 09:13
@He-Pin
He-Pin force-pushed the fix/objextend-nonobject-lhs branch 2 times, most recently from bf763b2 to 8284ca9 Compare September 3, 2026 09:25
Motivation:
Jsonnet defines lhs { body } as syntactic sugar for lhs + { body }.
Requiring an object lhs incorrectly rejected valid expressions such as
"hello" { x: 1 }, which should stringify the object and concatenate it.

Modification:
Preserve the object-inheritance fast path. Delegate non-object operands
to BinaryOp + using the already evaluated lhs, sharing RHS construction,
stringification, and diagnostics without repeating lhs evaluation.
Add regression tests for operand types, object bodies, evaluation order,
laziness, inheritance, shared body caches, and foldl fallback, including
negative goldens and 14 success assertions.

Result:
Object extension follows addition semantics while retaining the existing
strict-mode syntax restriction. All three JVM Scala versions pass 677
tests each (2031 total). Formatting, assembly, and CLI golden checks pass.
The success assertions also pass on Go Jsonnet 0.21.0 and 0.22.0.

References:
https://jsonnet.org/ref/spec.html#desugaring
deltarocks/jrsonnet@b5fcc26
@He-Pin
He-Pin force-pushed the fix/objextend-nonobject-lhs branch from 8284ca9 to 7e6e21b Compare September 11, 2026 14:40
@He-Pin He-Pin changed the title fix: ObjExtend should work with non-object lhs fix: apply addition semantics to object extension Sep 11, 2026
@He-Pin
He-Pin marked this pull request as ready for review September 12, 2026 05:54
He-Pin added a commit to He-Pin/sjsonnet that referenced this pull request Sep 12, 2026
Motivation:
The non-object extension fallback allocated a synthetic BinaryOp and dispatched
the already evaluated base again. Sharing addition must preserve ordinary
addition performance as well as Jsonnet values, evaluation order, and errors.

Modification:
Extract the original tuple-based addition arms into shared helpers. Keep numeric
addition and string concatenation in a small common helper that HotSpot can
inline, and retain the object-inheritance fast path and defensive ObjBody case.
Cover both shared-body cache orders and NaN rejection, and add JMH controls for
cached and allocating addition, string concatenation, and object extension.

Result:
All 2,034 JVM tests pass across Scala 2.12.21, 2.13.18, and 3.3.8. The 52 semantic
cases retain identical stdout, stderr, and exit codes in every version. Formatting
and all 52 benchmark regression fixtures pass (126 completed Mill tasks).
String/object extension allocates 32 fewer bytes per operation in the isolated
JMH workload. Interleaved addition controls show no stable regression; overall
latency improvement is not established. This refactor can be reverted separately
from the object-extension correctness fix.

References:
databricks#1120
https://jsonnet.org/ref/spec.html#desugaring
@He-Pin
He-Pin marked this pull request as draft September 12, 2026 08:38
@He-Pin
He-Pin force-pushed the fix/objextend-nonobject-lhs branch from a2434ab to 7e6e21b Compare September 12, 2026 08:38
@He-Pin
He-Pin marked this pull request as ready for review September 12, 2026 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant