padl / padr: optional pad-char arg for zero-pad / dot-leader#303
Merged
Conversation
cebd7fb to
14005bd
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
14005bd to
b9088f3
Compare
Two-instruction sequence carrying a 4th register in the data word for the pad-char operand. Mirrors the existing OP_SLC / OP_CLAMP / OP_RGXSUB shape so find_block_leaders and the JIT prefetcher already know how to skip the trailing data word. Pad char must be a 1-Unicode-scalar string; multi-char, empty, and non-text raise a Type error with span. Default behaviour for the 2-arg form is unchanged (OP_PADL / OP_PADR still emit a 3-register instruction with no data word, padding with spaces). Includes JIT helpers (jit_padlc / jit_padrc) for the Cranelift bridge and 5 lib tests covering happy path, multichar reject, non-string reject, and a Unicode scalar pad char.
Wires the new opcodes through both Cranelift compilers (JIT for --run-cranelift, ObjectModule for ahead-of-time). Both need: a helper FuncId on HelperFuncs, declare_helper for the 4-arg signature, the opcode added to the non-numeric output classifier, the opcode pair in the dispatch loop with skip_next=true so the data-word instruction isn't decoded as an opcode, and (JIT only) symbol registration so JITBuilder can resolve jit_padlc / jit_padrc at runtime. AOT picks up the symbols from libilo.a via the standard linker because jit_padlc / jit_padrc are exposed with #[unsafe(no_mangle)] pub(crate) extern "C".
Extends the tree-walker's padl / padr branch to accept 2 or 3 args. Third arg, when present, must be a 1-Unicode-scalar string; anything else is an ILO-R009 with the same wording the JIT helper uses for the analogous failure modes. Default 2-arg behaviour (pad with space) is unchanged.
Adds padl / padr to the existing arity-overload list (get / rd / wr / post / map / fld) so calls with 3 args type-check. Third arg, when present, must be compatible with t; an incompatible type raises ILO-T013 with arg-3-specific wording.
Seven new tests run on every engine (tree, VM, Cranelift) covering: - zero-pad numeric (the persona-reported use case) - right-pad with dots - already-wider short-circuit with custom pad char - Unicode scalar pad char (catches byte-vs-char confusion) - multi-char pad reject (tree + VM error; Cranelift returns nil per the same engine-divergence as the negative-width case above) - empty pad string reject - 2-arg form still pads with space (back-compat lock-in)
SPEC.md gets two new rows for the 3-arg overload with example pad chars (0 for sortable zero-padded keys, . for dot-leader alignment). examples/pad.ilo gets zp and dr helpers plus updated row to show zero-padded numeric output; tests/examples_engines.rs picks them up automatically and runs the new -- run / -- out assertions on every engine. ai.txt and skills/ilo/SKILL.md regenerate from SPEC.md via build.rs; both are git-tracked and idempotence-checked in CI.
b9088f3 to
1ccbc61
Compare
2 tasks
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.
Summary
padlandpadrpreviously hard-coded space as the pad char. That makes thecommon zero-padding pattern (sortable numeric keys, fixed-column log lines)
clunky enough that personas reach for
cat+ manual string-building, whichcosts tokens and retries. This adds an optional 3rd arg:
padl s w pc/padr s w pc, pad char defaults to space when omitted.Manifesto framing: the cost of a missing 3rd arg here was one extra builtin
call and a few lines of glue per occurrence, multiplied by every persona who
hit it. Adding it costs two opcodes, four lines on the SPEC table, and zero
back-compat risk because the 2-arg form keeps its existing bytecode shape.
Repro
Before:
After:
What is in the diff
Six commits, one per layer so each is reviewable on its own:
OP_PADLC/OP_PADRC(167 / 168) using the existing 2-instructiondata-word encoding (precedent:
OP_SLC,OP_CLAMP,OP_RGXSUB). Added tofind_block_leaders. Newjit_padlc/jit_padrchelpers + 5 lib unit testscovering happy path, multichar reject, non-string reject, Unicode scalar.
compile_cranelift.rs(AOT) andjit_cranelift.rs(JIT): helper FuncId, declaration, classifier list,dispatch +
skip_next, JIT symbol registration. AOT picks the symbols upfrom
libilo.avia the linker.1-Unicode-scalar string with
ILO-R009.padl/padrjoined to the arity-overload list. Third argtype-checked against
t(ILO-T013if it isn't).already-wider, Unicode scalar, multichar reject, empty reject, 2-arg
back-compat.
examples/pad.ilowith new-- run/-- outassertions (picked up by
tests/examples_engines.rs).ai.txtandskills/ilo/SKILL.mdregenerated from SPEC.md viabuild.rs.Test plan
cargo test --release --features cranelift(5215 passed, 0 failed, 45 ignored)cargo clippy --release --features cranelift --lib -- -W clippy::allcleantests/regression_pad.rs16/16 across tree, VM, Cranelift JITtarget/release/libilo.aexamples/pad.ilo-- run/-- outassertions pass on every enginerepeat_nmodernisation, identical-block merge, SAFETY comment on newas_heap_ref()Follow-ups
None. Cranelift's nil-on-bad-input behaviour for pad-char (mirrors the
existing engine-divergence on negative width) is the same gap the existing
pad_negative_width_errorstest already calls out as a deferred follow-up;harmonising the two together is out of scope for this PR.