Jens-G opened a new pull request, #3916:
URL: https://github.com/apache/thrift/pull/3916
Independent of the other Smalltalk PRs — this is the compiler, they are the
library.
The generator expanded nested struct serialization **textually**: generating
the code for a struct-typed field emitted that struct's whole reader and writer
inline, at every use. No base case, no visited set. A type containing itself
expanded for ever and the generator recursed until its own stack ran out.
```
$ thrift --gen st test/Recursive.thrift
Segmentation fault (core dumped)
```
Same for the minimal self-recursive struct in the issue, and for mutually
recursive types.
## The change
Each generated struct class now carries its own serialization — `writeTo:
oprot` on the instance side, `readFrom: iprot` on the class side. A
struct-typed field compiles to a *call*, not a copy of the body, so the
generator visits each struct once.
The bodies are the same code as before: `struct_writer`/`struct_reader` are
unchanged, just emitted once per struct instead of at every use. The
args/result structs a service synthesises stay inlined — they have no class to
hang a method on — and they no longer expand without bound, because expansion
now stops at the first declared struct.
## This is what makes the depth limit real
Inlined, the `incrementRecursionDepth` pairs counted **lexical** nesting,
fixed when the code was generated. Now each struct counts one level per
**actual call**, so THRIFT-6052's limit applies to how deep the *data* is.
Verified on a self-recursive type:
```
depth 60 -> accepted | depth 64 -> accepted
depth 65 -> rejected: 'Maximum recursion depth exceeded'
depth 200 -> rejected: 'Maximum recursion depth exceeded'
```
That could not be tested at all before, because a recursive type could not
be generated. It is also why `TProtocolRecursionDepthTest` uses a chain of 65
distinct types; its header said so, and this PR corrects that comment.
Rewriting the suite to use a genuinely recursive type is worth doing and is a
separate change.
## Two things the restructure forced
**`write_val` wrote base types and enums to `iprot`** while every other part
of the write path used `oprot`. That worked only because `TClient>>inProtocol:`
defaults `oprot` to the same object — anyone calling `outProtocol:` got field
values written to the *input* protocol. Inside `writeTo: oprot` there is no
`iprot` at all, so these now say `oprot`. (The recv method still mixes the two
for message framing; that is untouched here and worth its own ticket.)
**`read_val`'s struct case is parenthesised.** It now returns a keyword
message rather than a block, and `coll add: Foo readFrom: iprot` would
otherwise parse as a single `add:readFrom:` send. I hit exactly that on the
first build.
## Verification
Regression guard: **`st_recursive_types`**, a compiler test running `--gen
st` over `test/Recursive.thrift`. Deliberately no `PASS_REGULAR_EXPRESSION` —
it passes only on a zero exit, and a signal is not one. It runs in the existing
`compiler` ctest job, on Linux and Windows.
```
before: Test #15: st_recursive_types ... ***Exception: SegFault
after: Test #15: st_recursive_types ... Passed
```
Then, in Pharo against the actual generated code:
| | |
|---|---|
| self-recursive type generates, files in, round-trips a 5-deep value |
depth 5 in, depth 5 out, innermost item preserved |
| existing suites on code from the new generator | recursion-depth **6/6**,
string-size-limit **11/11** |
| non-recursive nested struct, old vs new generator | **byte-for-byte
identical** wire output |
That last one is the one I would want a reviewer to lean on: the same
`Outer{Inner{a:42}, name:'hi'}` through `sendEchoO:` produces exactly the same
41 bytes before and after.
## Note
**Generated Smalltalk changes shape for everyone** — struct classes gain two
methods, service methods get shorter. Regeneration is normal, but it is a
visible difference.
JIRA: [THRIFT-6062](https://issues.apache.org/jira/browse/THRIFT-6062)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
