fix: fmt rejects printf-style format specs instead of silently passing them through#297
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
danieljohnmorris
force-pushed
the
fix/fmt-format-spec
branch
3 times, most recently
from
5a90e6f to
37da4be
Compare
fmt's placeholder scanner matched only the exact pair `{}` and let every
other `{...}` sequence fall through as literal text. So `fmt "{:06d}" 42`
returned the string `"{:06d}"` rather than `"000042"`, with no warning.
Personas reaching for Python format specs got silent wrong output and
debugging time.
Now `{:...}` raises ILO-R009 from the interpreter, with a hint pointing
at `fmt2 v 2` for decimal precision and `padl (str n) 6` for padding.
Single source of truth for tree, VM, and Cranelift (via the tree-bridge).
When the template is a string literal we can catch the unsupported spec at verify time, before the program ever runs. This is the dominant case (personas type the format string directly), so failing early saves the compile/run round-trip and surfaces the hint at the call site. Mirrors the runtime error message and hint, so the diagnostic is the same whether the verifier or the interpreter catches it.
jit_call_builtin_tree swallows every bridge RuntimeError as TAG_NIL by
design, deferring typed propagation to the nil-sweep series. That leaves
Cranelift diverging from tree and VM: tree/VM raise ILO-R009 on
`fmt "{:06d}" 42`, Cranelift silently returns nil and the program
continues with corrupt data.
Scope this narrowly to `Builtin::Fmt` so the surrounding nil-sweep
contract is preserved. Cranelift now sets JIT_RUNTIME_ERROR on a fmt
error and renders the same diagnostic as tree/VM. Wider promotion of
the bridge's error channel stays on the nil-sweep roadmap.
15 cases across tree, VM, and Cranelift covering:
- verify-time ILO-T013 on literal `{:06d}` and `{:.3f}` templates
- runtime ILO-R009 when the template is computed at runtime
- bare `{}` still works
- lone `{` followed by non-`:` non-`}` stays a literal (JSON-like text)
Shows the two idiomatic shapes for callers who arrived at fmt looking
for printf-style specs:
fmt "...{}" (fmt2 v 2) -- decimal precision
padl (str n) 6 -- width / padding (space-pad)
Exercised across every engine by tests/examples_engines.rs.
Each surface that documents fmt now notes the bare-`{}`-only contract
and points at the supported substitutes (`fmt2` for decimal precision,
`padl` for padding). Stops future agents from re-discovering the same
silent-wrong-output footgun.
danieljohnmorris
force-pushed
the
fix/fmt-format-spec
branch
from
37da4be to
e204891
Compare
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
1 participant
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.
Summary
fmtparsed{}as a placeholder but every richer{...}sequence fell through char-by-char as literal text, sofmt "{:06d}" 42returned the string"{:06d}". Every persona reaching for Python-style format specs (zero-pad sort keys, decimal precision, etc.) got silent wrong output and minutes of debugging.Manifesto framing: a silent wrong string corrupts every downstream token in the program; an explicit error costs one retry. Composing existing builtins (
fmt2for precision,padlfor padding) keeps the surface area small.Repro before/after
Before:
x={:06d}(silent wrong output)After:
What's in the diff (per commit)
{:...}specs. Single source of truth for tree, VM (via tree-bridge), and Cranelift.jit_call_builtin_treesurfacesFmtbridge errors throughJIT_RUNTIME_ERRORso Cranelift renders the same diagnostic as tree/VM. Scoped narrowly toBuiltin::Fmtto preserve the surrounding nil-sweep contract.{}regression guard, and lone{JSON-like text.examples/fmt-format-spec.iloshows the idiomatic substitutes so future agents see the supported shape.{}contract.Test plan
cargo build --release --features cranelift(clean)cargo fmt --check(clean)cargo clippy --release --features cranelift --all-targets -- -D warnings(clean)cargo test --release --features cranelift --test regression_fmt_format_spec(15/15)cargo test --release --features cranelift --test examples_engines(1131/1131)cargo test --release --features cranelift(full suite, all 127 binaries green)--run-tree,--run-vm,--run-craneliftFollow-ups
Fmt) is on the nil-sweep roadmap, not blocking here.padlaccepting an explicit pad char (padl s w c) is the natural way to express zero-pad and was noted as#padl-padcharin the assessment doc; deferred to its own PR.