diff --git a/PLAN.md b/PLAN.md index 7c0ece341fb394cc320597d26bf5c2a1a534c93b..51489b02fa12a7f62195d12a05f7e47b22d8158d 100644 --- a/PLAN.md +++ b/PLAN.md @@ -151,7 +151,7 @@ **Done.** The loopback transport runs each streaming handler on its own fiber and bridges client ↔ handler via `fiber.channel`. All four flavors (unary + 3 streaming) are exercised by parameterized luatest groups. -### M5 — Conformance + interop *(proto3 closed; CI wire-up + proto2 conformance deferred)* +### M5 — Conformance + interop *(proto2 + proto3 both closed; CI wire-up still pending)* - [x] Wire up Google's [protobuf conformance test runner][conformance]. `cmd/conformance-runner.lua` is the testee: reads length-prefixed @@ -177,12 +177,13 @@ Current baseline: - Binary+JSON suite: **1493 ✓ / 1313 skipped / 0 failures** - Text-format suite: **416 ✓ / 18 skipped / 0 failures** - The 1313 + 18 skipped all target - `protobuf_test_messages.proto2.TestAllTypesProto2`. Proto2 codegen - itself is now supported for the core feature set (M9 below), but - that schema also needs `extend`/`extensions`/`group`/`MessageSet` - which we don't implement — see - [docs/codegen.md#proto2-support](../docs/codegen.md) for the gap. + Both proto2 and proto3 schemas now run through the harness: + Binary + JSON 2806 ✓ / 0 failures; Text-format 434 ✓ / 0 + failures (was 1493 + 416 ✓ with proto2 fully skipped). The four + MessageSet-flavored nested messages remain stripped from our + vendored copy of `test_messages_proto2.proto` because + protobuf-go's protoreflect rejects `message_set_wire_format`; + see [docs/codegen.md#proto2-support](../docs/codegen.md). Strict-validation closures landed across three commits on the `text-conformance-output` branch: @@ -387,7 +388,7 @@ `require('pb')`. - [ ] Example apps: pet-clinic CRUD over gRPC; replication of state via protobuf-encoded events on a queue. -### M9 — Proto2 baseline *(core done; conformance gated on legacy features)* +### M9 — Proto2 baseline *(done; 100% conformance on the patched proto2 suite)* - [x] Plugin accepts `syntax = "proto2"`. Field descriptors now carry `required = true`, `default_value = …`, and `optional = true` for @@ -401,23 +402,45 @@ - [x] Codec: `required` writer errors on missing-on-encode in both inline and runtime modes. - [x] JSON + text codecs: presence-tracked elision (`required`, `optional`, oneof) bypasses the proto3 implicit-zero rules. -- [x] 40 luatest cases (`proto2_basic.*` groups) — full, runtime, and - dynamic-from-source byte parity. -- [ ] **`extend` blocks / `extensions` ranges.** Currently extensions - survive as `_unknown_fields` bytes (so re-encode is lossless), - but the plugin doesn't emit accessors. Needed for the proto2 - conformance schema. -- [ ] **`group` fields** (legacy `optional group Foo = 1 { ... }`). - Wire types 3/4 SGROUP/EGROUP aren't in the encode/decode tables; - `protoreflect.GroupKind` falls off `scalarName` so any schema - containing a group field fails to generate. +- [x] **`group` fields** end-to-end. Wire layer carries + `WIRE_SGROUP=3` / `WIRE_EGROUP=4`; the codec's + `decode_group(desc, buf, pos, stop_id)` mirrors decode_message + but stops on EGROUP. Inline + runtime codegen both emit start/end + tag pairs around the body (no LEN prefix). Text format renders + groups under the capitalized submessage name. Runtime parser + desugars `optional|required|repeated group Name = id { … }` into + a nested message plus a `kind='group'` field whose lowercased + name is `name` — matches build-time codegen for the dynamic path. +- [x] **`extend` blocks / `extensions` ranges.** Plugin iterates + `file.Extensions` + per-message `m.Extensions`, emitting + `pb.register_extension(extendee_desc, {…})` calls. The codec + stores set extensions under `data._extensions[full_name]` and + walks them after the regular field loop on encode; decode routes + tags not in `field_by_id` through `decode_extension` when + `extensions_by_id` matches. Text + JSON encoders/decoders speak + the bracketed `[pkg.ext_name]` syntax. Meta-extensions on + `google.protobuf.*` descriptors (file/message/field options) are + skipped at codegen time — the WKT module doesn't surface those + descriptors at runtime. +- [x] **Proto2 closed enums.** Plugin emits `closed = true` on every + proto2 enum descriptor (driven by protoreflect's `IsClosed()`). + Text format's `parse_enum_value` rejects integer literals that + don't map to a declared value for closed enums; proto3 stays + open to preserve forward-compatibility on the wire. +- [x] 50+ luatest cases (`proto2_basic.*` groups) — full, runtime, and + dynamic-from-source byte parity, plus group wire-byte pinning + and JSON/text presence rules. - [ ] **`MessageSet`** wire format. `protobuf-go`'s protoreflect refuses to load a FileDescriptor declaring `option message_set_wire_format = true;`, calling it a removed - proto1 feature. Can't bypass without forking the library. -- [ ] **Conformance numbers.** All 1313 + 18 currently-skipped tests - target `TestAllTypesProto2`, which exercises all three of the - above. Won't move until at least extend + group are wired up. + proto1 feature. We strip the four MessageSet-flavored nested + messages from our vendored + `test/conformance/proto/test_messages_proto2.proto` so the rest + of the schema compiles. Real support would require forking + protoreflect; deferred until a real Tarantool consumer asks. +- [x] **Conformance numbers.** Full suite, strict mode (protobuf v34.1): + Binary + JSON 2806 ✓ / 0 failures (was 1493 / 1313 skipped); + Text-format 434 ✓ / 0 failures (was 416 / 18 skipped). ## 4. Per-feature design notes diff --git a/README.md b/README.md index afc9c91d99974148012f774cceffac8a92bbfc05..24f5edfe741c688af9eb1d39a71fba780a22aa24 100644 --- a/README.md +++ b/README.md @@ -15,8 +15,15 @@ with Tarantool's built-in module. ## Status -Proto3 conformance is closed: every `Required.*` and `Recommended.*` test in -both the binary+JSON and text-format suites passes. +Proto2 + proto3 conformance is closed: every `Required.*` and `Recommended.*` +test in both the binary+JSON and text-format suites passes. The proto2 +slice covers `required`/`optional`/`repeated` cardinalities, custom +defaults, closed enums, legacy `group` fields (SGROUP/EGROUP wire types), +and extensions (`extend`/`extensions`). The only un-runnable upstream +schemas are those with `option message_set_wire_format = true;` — the +protobuf-go protoreflect library rejects them outright as a removed +proto1 feature, so the MessageSet fixture is stripped from our vendored +copy of `test_messages_proto2.proto`. | Feature | State | |----------------------------------|--------------| @@ -41,8 +48,8 @@ | WKT: Struct, Value, ListValue | ✅ | | WKT: Any (opaque + registry pack/unpack) | ✅ | | WKT: FieldMask (strict round-trip) | ✅ | | Byte-for-byte interop with `protoc` (10 fixtures) | ✅ | -| **Google conformance suite — proto3 binary+JSON** | **1493 ✓ / 0 failures** | -| **Google conformance suite — proto3 text format** | **416 ✓ / 0 failures** | +| **Google conformance suite — binary + JSON** | **2806 ✓ / 0 failures** | +| **Google conformance suite — text format** | **434 ✓ / 0 failures** | | Runtime `.proto` parsing (`pb.parse`) | ✅ | | Runtime `FileDescriptorSet` ingest (`pb.from_pb`) | ✅ | | Markdown doc generator (`protoc-gen-tarantool-doc`) | ✅ | @@ -52,10 +59,11 @@ | Unknown-field passthrough (`_unknown_fields`) | ✅ | | Microbenchmark + alloc regression gate (`just bench`) | ✅ | | proto2 `required` / `optional` / custom `[default = X]` | ✅ | | proto2 repeated unpacked-by-default + `[packed = true]` | ✅ | -| proto2 extensions / `extend` blocks | ❌ (passes through as unknown fields) | -| proto2 groups (legacy) | ❌ (parse rejected; no wire-type 3/4 codec) | -| Google conformance suite — proto2 | ❌ blocked on groups + extensions + MessageSet | -| Editions | ❌ deferred | +| proto2 `group` (SGROUP/EGROUP wire types) | ✅ | +| proto2 extensions / `extend` blocks | ✅ | +| proto2 closed enums | ✅ | +| `MessageSet` wire format | ❌ (protoreflect rejects upstream — `message_set_wire_format` option) | +| Editions | ❌ deferred | ## Install @@ -356,21 +364,19 @@ `test/conformance/known_failures.txt` (binary + JSON suite) and `test/conformance/known_failures_text.txt` (text-format suite); both are empty for the proto3 suites as of 2026-05-16. -Current baseline (2026-05-16, protobuf v34.1): +Current baseline (2026-05-17, protobuf v34.1, `--enforce_recommended`): | Suite | Successes | Skipped | Expected failures | Unexpected | |-------|-----------|---------|-------------------|------------| -| Binary + JSON | **1493** | 1313 | 0 | 0 | -| Text-format | **416** | 18 | 0 | 0 | +| Binary + JSON | **2806** | 0 | 0 | 0 | +| Text-format | **434** | 0 | 0 | 0 | -The 1313 + 18 skipped tests all target -`protobuf_test_messages.proto2.TestAllTypesProto2`. Proto2 codegen now -works for the core feature set (see Status table), but the conformance -schema relies on extensions (`extend`), `extensions` ranges, legacy -`group` fields, and a `MessageSetCorrect` that protoreflect rejects -outright as a removed proto1 feature. Until those are wired up the -proto2 suite stays at zero — see -[docs/codegen.md#proto2-support](docs/codegen.md) for the scope. +Both proto2 and proto3 test messages run through the same dispatcher in +`cmd/conformance/core.lua`. The `test_messages_proto2.proto` checked +into `test/conformance/proto/` is a vendored copy with the +`MessageSetCorrect*` nested messages stripped (see +[docs/codegen.md#proto2-support](docs/codegen.md)); everything else +matches upstream. The runner supports `protobuf_test_messages.proto3.TestAllTypesProto3` in binary, JSON, and text-format input/output, including the diff --git a/docs/codegen.md b/docs/codegen.md index 2fcc09b1494dad4da80066a2adc774b6cf5e6a27..b2483b28eda65da30cae9052a5e106ff9e692d21 100644 --- a/docs/codegen.md +++ b/docs/codegen.md @@ -126,9 +126,13 @@ {name='x', id=1, kind='scalar', proto_type='int32'}, {name='y', id=2, kind='message', message=}, {name='z', id=3, kind='enum', enum=}, {name='m', id=4, kind='map', key=, value=}, + {name='g', id=5, kind='group', message=}, -- modifiers: repeated, packed, oneof, optional, -- required (proto2), default_value (proto2) }, + -- Proto2-only, optional: + extensions_by_id = {[N] = , ...}, + extensions_by_full_name = {['pkg.ext_name'] = , ...}, -- Filled in by pb.finalize_message: field_by_id = {[1]=, ...}, field_by_name = {x=, ...}, @@ -370,30 +374,67 @@ `proto2_basic.*` luatest groups (40 cases) cover defaults, required validation, the unpacked-by-default rule, nested-required messages, and full-vs-runtime-vs-dynamic byte parity. -### What still doesn't work +### Groups + +Proto2 `group` fields (legacy `optional group Foo = 1 { … }`) carry +their own wire format: SGROUP (wire type 3) opens, the body follows +as regular field encodings, and EGROUP (4) with the matching field +id closes. No length prefix. The codec exposes +`pb.codec.decode_group(desc, buf, pos, stop_id)` which reuses the +per-tag dispatch from `decode_message` but bails on EGROUP instead +of end-of-buffer. Inline codegen emits a single `kind='group'` field +that brackets its body with start/end tags; repeated groups wrap +each element in its own pair. + +Text format uses the group's submessage simple name as the label +(`Data { … }`, `MultiWordGroupField { … }`) and accepts the +lowercase ASCII fold (`data { … }`); the `:` between label and `{` +is optional, matching mainline protoc. + +### Extensions + +`extend Foo { optional Bar baz = N; }` declarations register an +extra tag on `Foo`. The plugin emits +`pb.register_extension(M.Foo_descriptor, {name='baz', full_name='pkg.baz', id=N, …})` +calls at module load time. The runtime attaches two indices to the +extendee — `extensions_by_id` (decode lookup) and +`extensions_by_full_name` (encode + textual emission order). + +Set extension values live on a message under `_extensions`: + +```lua +local m = {_extensions = {['pkg.baz'] = {nested = 7}}} +local bytes = M.Foo_encode(m) +local back = M.Foo_decode(bytes) +back._extensions['pkg.baz'] -- {nested = 7} +``` + +Wire bytes for set extensions interleave with regular fields (the +codec walks `extensions_by_full_name` after the field loop on encode +and routes unknown tags to `decode_extension` on the way in). Text +format speaks the bracketed `[pkg.baz]: value` (or `[pkg.baz] { … }` +for message/group extensions). JSON uses the same bracketed key +form per the proto2 JSON spec. Extensions extending +`google.protobuf.*` descriptors (file/message/field options) are +skipped at codegen — those are meta-only and the WKT module doesn't +surface their descriptors at runtime. -The conformance schema (`test_messages_proto2.proto` from upstream -protobuf) leans on three features we don't implement: +### Closed enums -- **`extend` blocks** + **`extensions 100 to 199;`** ranges — proto2's - open-message-extension mechanism. Extension values currently - round-trip through `_unknown_fields` like any other unknown tag, - so a message carrying an extension can be decoded → re-encoded - byte-identically. But the plugin won't emit accessors and there's - no way to read or set extension values from user code. -- **`group`** fields (legacy `optional group Foo = 1 { … }`) — uses - wire types 3 (SGROUP) and 4 (EGROUP) which our wire layer doesn't - encode or decode. We skip-tolerate them on the read side but - generating a message that *contains* a group field fails because - `protoreflect.GroupKind` falls off our `scalarName` switch. -- **`MessageSet`** wire format — `protobuf-go` rejects messages - declaring `option message_set_wire_format = true;` at the - protoreflect layer ("a legacy proto1 feature that is no longer - supported"). We can't bypass this without forking protoreflect. +Proto2 enums are closed: a numeric literal that doesn't match any +declared value is a parse error in text format (and on JSON via the +same path). The plugin emits `closed = true` on every proto2 enum +descriptor (driven by protoreflect's `IsClosed()`). Proto3 enums +stay open to preserve forward-compatibility on the wire. -Implementing extensions and groups is the realistic path to closing -the conformance gap; MessageSet would need an upstream workaround. -Both are big enough lifts to be separate slices — proto2 is mostly -legacy and the existing extension-via-unknown-fields path covers the -common interop case (you receive bytes, you re-encode them, the -extension data passes through intact). +### What still doesn't work + +- **`MessageSet`** wire format. `protobuf-go`'s protoreflect refuses + to load a FileDescriptor declaring `option message_set_wire_format + = true;`, calling it "a legacy proto1 feature that is no longer + supported". Our vendored copy of + `test/conformance/proto/test_messages_proto2.proto` has the four + MessageSet-flavored nested messages stripped so the rest of the + schema compiles — the patch is documented in the file's header. + Real MessageSet support would need to fork protoreflect; deferred + until a Tarantool consumer asks.