From e176c85bb726f04a3c2a2bc2440f3e7b1a988636 Mon Sep 17 00:00:00 2001 From: He-Pin Date: Tue, 12 May 2026 12:10:57 +0800 Subject: [PATCH] fix: keep std.trace lazy during static optimization Motivation: std.trace has an observable side effect, but the static optimizer treated it like a safe builtin and could execute it while folding expressions. That allowed traces from unused object fields or unused format expressions and could reorder trace output compared with official Jsonnet runtime behavior. Modification: Mark the std.trace builtin as not static-safe so it is evaluated only when the Jsonnet program forces it at runtime. Add regression tests that unused traces inside objects and format RHS values stay silent while a directly used trace still logs, and update the upstream trace golden to the runtime trace order. Result: Full ./mill --no-server --ticker false --color false -j 1 __.test passed with 438 tests. Three independent reviews found no blockers. --- .../src/sjsonnet/stdlib/StdLibModule.scala | 2 ++ .../resources/test_suite/trace.jsonnet.golden | 2 +- .../test/src/sjsonnet/EvaluatorTests.scala | 34 +++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/sjsonnet/src/sjsonnet/stdlib/StdLibModule.scala b/sjsonnet/src/sjsonnet/stdlib/StdLibModule.scala index 706d28fe2..fe59fc692 100644 --- a/sjsonnet/src/sjsonnet/stdlib/StdLibModule.scala +++ b/sjsonnet/src/sjsonnet/stdlib/StdLibModule.scala @@ -80,6 +80,8 @@ object StdLibModule { ) rest.value } + + override def staticSafe = false } /** diff --git a/sjsonnet/test/resources/test_suite/trace.jsonnet.golden b/sjsonnet/test/resources/test_suite/trace.jsonnet.golden index 859ee3426..3a81736d7 100644 --- a/sjsonnet/test/resources/test_suite/trace.jsonnet.golden +++ b/sjsonnet/test/resources/test_suite/trace.jsonnet.golden @@ -7,6 +7,6 @@ TRACE: trace.jsonnet TRACE: trace.jsonnet TRACE: trace.jsonnet TRACE: trace.jsonnet -TRACE: trace.jsonnet Some Trace Message TRACE: trace.jsonnet +TRACE: trace.jsonnet Some Trace Message true diff --git a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala index 57d613902..228a65d02 100644 --- a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala +++ b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala @@ -4,6 +4,23 @@ import utest._ import TestUtils.{eval, evalErr} object EvaluatorTests extends TestSuite { + private def evalWithTraces(s: String): (ujson.Value, Vector[String]) = { + var traces = Vector.empty[String] + val interpreter = new Interpreter( + Map(), + Map(), + DummyPath(), + Importer.empty, + parseCache = new DefaultParseCache, + logger = (isTrace, msg) => if (isTrace) traces :+= msg + ) + val result = interpreter.interpret(s, DummyPath("(memory)")) match { + case Right(value) => value + case Left(err) => throw new Exception(err) + } + (result, traces) + } + def tests: Tests = Tests { test("arithmetic") { eval("1 + 2 + 3") ==> ujson.Num(6) @@ -569,6 +586,23 @@ object EvaluatorTests extends TestSuite { eval("\"%()s %()s!\" % [\"Hello\", \"World\"]") ==> ujson .Str("Hello World!") } + test("trace laziness") { + val (unusedObj, unusedObjTraces) = evalWithTraces( + """local x = {a: std.trace("unused object", "ok")}; 0""" + ) + unusedObj ==> ujson.Num(0) + unusedObjTraces ==> Vector.empty + + val (unusedFormat, unusedFormatTraces) = evalWithTraces( + """local x = "%% %(a)s %%" % {a: std.trace("unused format", "ok")}; 0""" + ) + unusedFormat ==> ujson.Num(0) + unusedFormatTraces ==> Vector.empty + + val (used, usedTraces) = evalWithTraces("""std.trace("used trace", 1)""") + used ==> ujson.Num(1) + usedTraces ==> Vector("TRACE: (memory) used trace") + } test("binaryOps") { val ex = assertThrows[Exception]( eval("1 && 2")