From 51f196338ff5ff64768ab451182c7b04d79c840b Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sat, 16 May 2026 18:12:44 +0300 Subject: [PATCH] docs: handoff brief for the pb.text.decode slice Self-contained brief targeting the 408 proto3 TextFormatInput tests still skipped after the encoder + SGROUP work. Covers existing surface (text.lua encode-only, parser.lua schema parser, conformance dispatch), grammar buckets the suite exercises (10 categories with upstream test counts), files to touch, LuaJIT/cdata/box.NULL conventions, and a verifying-locally checklist. Suggested recursive-descent shape mirrors runtime/pb/parser.lua's cursor mechanics. No code changes; positions the next slice without committing to a specific implementation schedule. --- docs/text_format_parser_brief.md | 254 +++++++++++++++++++++++++++++++ 1 file changed, 254 insertions(+) create mode 100644 docs/text_format_parser_brief.md diff --git a/docs/text_format_parser_brief.md b/docs/text_format_parser_brief.md new file mode 100644 index 0000000000000000000000000000000000000000..d434feae11e935769dc42ebe955e5cf5c93c0e44 --- /dev/null +++ b/docs/text_format_parser_brief.md @@ -0,0 +1,254 @@ +# Text-format parser brief — `pb.text.decode` + +Handoff doc for the next slice of text-format conformance work. Goal: +turn the 408 proto3 `TextFormatInput.*` tests we currently `skipped` +into passes. Read this end-to-end before opening files. + +## What exists + +- `runtime/pb/text.lua` — encode-only proto3 text-format printer. + Walks descriptor fields and emits `name: value` / `name { ... }`, + honors `_unknown_fields` under `opts.print_unknown_fields`. WKT + emitters for Timestamp/Duration/Struct/Value/ListValue/Any/FieldMask/ + Empty/Wrappers all hook in via `desc.text(t, buf, depth)`. +- `cmd/conformance/core.lua` — TEXT input still returns + `{skipped = 'text-format input not supported'}`. PB and JSON inputs + dispatch into `pb.decode` / `pb.json.decode`. TEXT output is wired. +- `runtime/pb/parser.lua` — pure-Lua proto3 *schema* parser (different + artifact; parses `.proto` files into AST). Useful only as a style + reference for hand-written Lua recursive-descent. +- `test/text_test.lua` — encoder tests parameterized over both codegen + modes (`full`, `runtime`). Pattern to mirror for decoder tests. +- `test/conformance_test.lua` — unit regressions that pin each upstream + conformance case to a `core.handle_request` call. Cheaper than the + Docker round-trip for the inner dev loop. + +## Goal + +Add `pb.text.decode(desc, text) -> table` (mirroring +`pb.text.encode(desc, t, opts)`) so: + +1. `cmd/conformance/core.lua` stops skipping `text_payload` inputs, + instead calling `pb.text.decode(desc, req.text_payload)` and feeding + the result into the existing output dispatch. +2. The proto3 text suite under + `test/conformance/known_failures_text.txt` is empty *after* + parser-level conformance is closed (some Recommended-only edge + cases may legitimately fail and need an entry — fine, mark them). +3. `test/conformance_test.lua` grows one regression per scenario + bucket (see "Grammar coverage" below) so the inner loop doesn't + require Docker. + +Encoder is already complete for the conformance corpus; do **not** +expand encoder scope unless the conformance probe shows a specific +gap. The only encoder polish in scope: emit textproto-style list +short-form `[a, b, c]` for repeated scalars when `opts.list_short = +true`, so decoded text round-trips cleanly through encode. Default +stays one-line-per-element (the form the current corpus uses). + +## Acceptance criteria + +- `make test` stays green; the existing 509 unit tests don't regress. +- New unit tests under `test/text_decode_test.lua` cover each grammar + item in the table below, parameterized over both `full` and + `runtime` modes against the `hello.Person` and `proto3.TestAllTypes` + descriptors. +- New regressions under `test/conformance_test.lua` in a + `core.text_input_*` cluster, one per scenario bucket, pinned with + byte-exact `text_payload` strings cribbed from the upstream + conformance source (see "How to extract test payloads" below). +- Docker conformance under default `just conformance` settings: + binary+JSON suite stays at 1478 ✓; text-format suite climbs from + 8 ✓ / 426 skipped → ~416 ✓ / ~18 skipped (the residual ~18 are + proto2 message-type skips, unaffected by this work). +- `runtime/pb/text.lua` decoder respects the same conventions as the + rest of the runtime: LuaJIT `int64_t`/`uint64_t` cdata for 64-bit + ints; `box.NULL` for the `Value` WKT's `null_value`; `ipairs` / + `for i = 1, #t` on any hot path (`pairs()` over a hash NYIs the JIT + trace per CLAUDE.md). + +## Files to touch + +``` +runtime/pb/text.lua — add decode() and helpers; keep encode untouched +cmd/conformance/core.lua — drop the text_payload skip, call pb.text.decode +test/text_decode_test.lua — new, parameterized over modes +test/conformance_test.lua — add a core.text_input_ cluster +test/conformance/known_failures_text.txt + — empty (or only Recommended residuals) +PLAN.md — bump M7 baseline: 8 ✓ → ~416 ✓ +runtime/pb/init.lua — no surface change; pb.text already exposed +``` + +No codegen-side changes (`cmd/protoc-gen-tarantool/...`) — decode is +descriptor-driven, not inlined. + +## Grammar coverage (from a `--verbose` skip-name extract) + +The conformance suite stresses these grammar features. Cover each +under both the unit-test set and the conformance regression cluster. +Counts are upstream-test pair counts (input is duplicated × output +formats). + +| Bucket | Count | Grammar feature | +|---|---:|---| +| Number literals | ~24 | decimal, hex (`0x…`), octal (`0…` prefix), trailing `f`/`F` for floats, signed exponents (`1e-50`, `1e9999`); `Int{32,64}/Uint{32,64}FieldMaxValue{Octal,Hex,}` all radixes | +| Float specials | ~50 | `inf`, `infinity`, `nan` in arbitrary casings (`NaN`, `iNF`, `INFINITY`); oversize exponents parse to ±inf; `Neg{Float,Double}LargeNegativeExponentParsesAsNegZero` (`-1e-50` → `-0.0`) | +| String/bytes literals | ~24 | `"…"` and `'…'`; escapes `\a \b \f \n \r \t \v \? \\ \' \"`; octal `\341`, hex `\xc0`, short-unicode `ሴ`, long-unicode `\U00010437`; adjacent-literal concat (`"a" "b"` → `"ab"`); UTF-8 validation for `string` fields (reject surrogates, overlongs); `bytes` accepts any byte | +| Aggregates | ~30 | `field: { ... }` and `field: < ... >` (legacy angle-bracket); fields separated by `;`, `,`, or whitespace; trailing `;` accepted; **multiple trailing** `;;` accepted (a corner case the suite tests) | +| Repeated short-form lists | ~30 | `repeated_int32: [123, 456]`, with `,` separator (and trailing `;` after the list). `[a b]` space-only is **invalid**; `[a; b]` semicolons inside lists is **invalid** | +| ReservedFieldName | 20 | any value shape after a reserved-field name (literal, list, message, group-style) must parse and be **silently dropped** — not produce parse_error | +| Map entries | ~8 | `map__ { key: K value: V }` aggregate form, one entry per occurrence | +| Any-typed inline | ~12 | `[type.googleapis.com/] { ... }` resolves to an Any with the inner message bytes; the URL syntax inside square brackets is the type-tag | +| Enums | ~4 | accept enum-by-name and enum-by-integer; unknown enum *name* must reject (proto3 rule) | +| Map key types | ~3 | bool keys (`key: true`), int keys, string keys | + +A scenario worth flagging: `ListSeparatorMissingIsOneValue_*` — +`field: [123]` followed by a *separate* `field: [456]` produces a +two-element repeated list. Don't collapse the two entries; they're +distinct list-shorts that each append. + +## How to extract test payloads + +For unit-test regressions, paste the byte-exact `text_payload` from +the upstream conformance source rather than reconstructing: + +```bash +# All proto3 text-input test names + their payloads: +docker run --rm -v "$(pwd):/work" -w /work \ + --entrypoint conformance_test_runner \ + --enforce_recommended --verbose \ + --failure_list test/conformance/known_failures.txt \ + --text_format_failure_list test/conformance/known_failures_text.txt \ + /usr/bin/tarantool cmd/conformance-runner.lua 2>&1 \ + | grep '^SKIPPED.*Proto3.*TextFormatInput' +``` + +Each line carries `test=, request=goo.gle/debugproto `. The payload after +`text_payload: "` is the input text (C-escaped); unescape it for the +Lua string literal. + +Alternatively, `--test --debug` dumps the octal-escaped request +for a single test, which is easier to copy verbatim. + +## Constraints / conventions + +- **No `pairs()` on hot paths** — the proto3 spec lets fields appear + in any order, but the parser's outer loop is `while pos < len do` + over the input string, not iteration over a hash. Field lookup + inside the loop must be `field_by_name[…]` (already O(1) hash + lookup, but only one per field-occurrence, not per byte). Map + fields are the one accepted exception per CLAUDE.md. +- **int64 family stays cdata.** `decode_uint64_token` returns + `ffi.cast('uint64_t', …)`, not a Lua number. The runtime's `wire.lua` + has `to_uint64` / `to_int64` helpers — use them for big integer + literals (esp. `0xFFFFFFFFFFFFFFFF` and `18446744073709551615`). +- **`box.NULL` for nulls.** When the parser encounters `null_value: + NULL_VALUE` inside a Value WKT, emit `box.NULL`, not `nil` / not a + custom sentinel — `runtime/pb/wkt.lua` already does this convention. +- **Numeric-field-id mode is optional** — protoc's + `AllowFieldNumber(true)` is what lets the harness re-parse our + output. For the parser, accept numeric field IDs by default since + the conformance harness emits some scenarios that way (e.g. + unknown-field text round-trips). Skip silently if the ID isn't in + the schema; same shape as `reserved` handling. +- **Token-recursion depth limits**: textproto recommends rejecting + deeply nested messages. A 100-level cap mirrors mainline protoc + and stops pathological inputs from blowing the Lua stack. +- **Reuse `runtime/pb/wire.lua` numeric edge logic.** Don't + re-derive int32-overflow rules in the parser — produce the value + as a uint64 cdata and feed through the same truncation helpers the + binary decoder uses (`wire.varint_to_int32`, etc.). + +## Suggested implementation outline + +Recommend recursive-descent, not a generated parser. The grammar is +small enough that a hand-written tokenizer + parser stays under ~600 +LOC. Mirror `runtime/pb/parser.lua` (the `.proto` schema parser) for +buffer/cursor mechanics: a `lexer` table with `pos`, `src`, `peek()`, +`expect(tok)`, plus skip-whitespace-and-comments helpers (textproto +supports `#` line comments; the conformance corpus tests at least one +comment scenario). + +Top-down phases: + +1. **Lexer** — number, string-literal (with all escape variants), + identifier, bracketed-Any URL, `:`, `{`, `}`, `<`, `>`, `[`, `]`, + `,`, `;`. Single function returning `(token_kind, token_value)` + and advancing `pos`. Skips whitespace + `#...\n` comments. +2. **Value parser** — given a field's `kind` + `proto_type`, consume + one value from the lexer. For scalars: type-specific token form + validation (e.g. floats accept `inf`/`nan`, ints don't). For + messages: open `{` or `<`, recurse, close. +3. **Field parser** — consume identifier, optional `:`, value, optional + trailing `;` or `,`. Look up by name in `desc.field_by_name`. If + field is `reserved` (need to thread reserved set into descriptors; + currently absent — see "Schema-side prep" below), parse-and-drop. + If unknown name and `opts.allow_unknown_fields = false`, error; + otherwise drop. +4. **Top-level entry** — `M.decode(desc, text)` constructs an empty + result, loops `field_parser` until input exhausted. + +WKTs need explicit overrides on the decode path too (Timestamp parses +RFC3339-ish forms when given as `seconds: N nanos: M`, Duration similar, +Value parses the oneof from token shape: `string_value: "..."` vs +`number_value: 1.5` vs `bool_value: true` vs `list_value { ... }` vs +`struct_value { ... }` vs `null_value: NULL_VALUE`). + +### Schema-side prep + +To pass `ReservedFieldName` we need to know reserved names. They're in +`FileDescriptorProto.MessageType[].ReservedName` (proto descriptor), +which our `cmd/protoc-gen-tarantool` plugin currently ignores. Plumb +`reserved_names = {…}` onto generated descriptors (both modes) and +expose `desc.is_reserved(name) -> bool` for the parser. Plugin change ++ regen via `make gen`. + +## Out of scope + +- Proto2 / editions message types. Even after this lands, ~1331 + proto2 tests stay skipped because we don't generate Lua for + `TestAllTypesProto2`. That's a separate axis: generate proto2 + bindings, register the descriptor in `MESSAGE_REGISTRY`, then ~1331 + more tests light up (most will likely pass since the wire format is + unchanged proto3-shape, but some proto2-specific shapes — + extensions, groups as a *first-class* feature, required-field rules + — need work). +- JSPB. Google-internal; nobody implements. +- TextFormat printer changes beyond the optional `list_short` opt. +- Streaming / incremental parse. The conformance corpus payloads are + small; one-shot string decode is fine. + +## Verifying locally + +Inner loop (no Docker): + +```bash +make gen && make test # full luatest suite, ~0.5s +# Spot-check one conformance test: +LUA_PATH="./runtime/?/init.lua;./runtime/?.lua;./examples/expected/?.lua;./examples/expected/?/init.lua;./cmd/?.lua;./cmd/?/init.lua;./test/?.lua;;" \ + .rocks/bin/luatest -v test/text_decode_test.lua +``` + +Full conformance (5–8 min): + +```bash +just conformance # both suites, both failure lists, --enforce_recommended +``` + +Single test (fast triage): + +```bash +docker run --rm -v "$(pwd):/work" -w /work \ + --entrypoint conformance_test_runner \ + tarantool-protobuf-conformance:latest --enforce_recommended \ + --test Required.Proto3.TextFormatInput.ReservedFieldName \ + --failure_list test/conformance/known_failures.txt \ + --text_format_failure_list test/conformance/known_failures_text.txt \ + /usr/bin/tarantool cmd/conformance-runner.lua +``` + +When the unit + parity tests are green and `just conformance` exits +zero, update `PLAN.md` with the new baseline and commit on top of +this branch (`text-conformance-output`).