From 4d7fda1f88cf269a302e54180b9e7dcf2e1ea878 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sat, 16 May 2026 20:02:36 +0300 Subject: [PATCH] docs: drop text-format parser handoff brief The pb.text.decode slice landed (7ec8f2b); the handoff brief is no longer load-bearing. Removes the file and unlinks references from README.md and PLAN.md. --- PLAN.md | 3 +- README.md | 2 - docs/text_format_parser_brief.md | 254 ------------------------------- 3 files changed, 1 insertion(+), 258 deletions(-) delete mode 100644 docs/text_format_parser_brief.md diff --git a/PLAN.md b/PLAN.md index da9d22e9aa4f14a5cbc0f47477f3c66c0445b40b..2d17a1b9561644c497c74c83322980769822acef 100644 --- a/PLAN.md +++ b/PLAN.md @@ -185,8 +185,7 @@ fiber and bridges client ↔ handler via `fiber.channel`. All four flavors Strict-validation closures landed across three commits on the `text-conformance-output` branch: - `pb.text.decode` — full grammar coverage (recursive-descent - parser, ~580 LOC; see - [docs/text_format_parser_brief.md](../docs/text_format_parser_brief.md)). + parser, ~580 LOC). - `codec` -0.0 preservation — float/double `is_default_scalar` and the inline-codegen elision both gained a sign-bit guard (`1/v == math.huge`). diff --git a/README.md b/README.md index 4e7a811ebff1a589359b67849986bf95b963a417..e35fba149e4a68007d1565816e50dba46b688d4c 100644 --- a/README.md +++ b/README.md @@ -155,8 +155,6 @@ bench/ per-helper bench + JIT-trace gate + alloc baseline - **[docs/codegen.md](docs/codegen.md)** — plugin internals, descriptor shape contract, how to add a new wire type or scalar, the LuaJIT hot-path rules the generated code observes. -- **[docs/text_format_parser_brief.md](docs/text_format_parser_brief.md)** — - retrospective brief on the text-format parser slice (`pb.text.decode`). - **PLAN.md** — phased roadmap and per-feature design notes. - **CLAUDE.md** — invariants and conventions enforced across the codebase (no `pairs()` on hot paths, 64-bit ints as cdata, SoA over AoS for large diff --git a/docs/text_format_parser_brief.md b/docs/text_format_parser_brief.md deleted file mode 100644 index d434feae11e935769dc42ebe955e5cf5c93c0e44..0000000000000000000000000000000000000000 --- a/docs/text_format_parser_brief.md +++ /dev/null @@ -1,254 +0,0 @@ -# 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`).