Shai Szulanski

Add shared-parser Thrift formatter binary Summary: Add the `thrift-fmt` binary and formatter library using the shared recursive-descent parser core for grammar validation and token collection from the first formatter diff. Formatter-specific trivia handling and layout remain in the formatter implementation; the semantic AST is not used as a formatting transport. Port the existing formatter snapshot coverage into C++ and add fixture round-trip/idempotence coverage plus the fbcode compatibility target. This implementation exactly preserves the behavior of the existing formatter, including all of its deficiencies, to facilitate deployment. The goal is to fix these issues individually and end up with a better formatter, using the flexibility of controlling the whole stack to make the behavior fully configurable while also eliminating divergence caused by parser duplication as the language evolves. Reviewed By: hchokshi, sadroeck Differential Revision: D108951643 fbshipit-source-id: d00e3ec3d0976ece612e7186c842257aa4ab0a2f

Add thrift key serializer Summary: Add `ThriftKeySerializer`, the canonical identity layer for object-key containers. It serializes thrift-form values to compact-protocol bytes so equal thrift values produce the same key regardless of object identity, while preserving list order, sorting unordered set/map contents recursively by serialized bytes, and writing structs/unions in field-id order. Adapters and wrappers are unwrapped through `ThriftSerializationHelper::unwrapApplyAdapter` so adapted Hack values compare by their thrift representation. Reviewed By: sadroeck, prakashgayasen, a-lafrance Differential Revision: D108350995 fbshipit-source-id: dc6ad842aa7a49f54a7b180adb3dbf982817694b

Deprecate const-qualified field_ref accessors Summary: Document shallow-const semantics for Thrift field-ref wrappers and steer callers to pass and capture these view objects by value without const qualifiers. Add non-const `value()`, `operator*()`, and `operator->()` overloads where needed so normal by-value use remains clean, deprecate the const-object overloads, and fix boxed/intern/union `as_const()` to preserve storage-handle constness while exposing const field values. Reviewed By: Mizuchi Differential Revision: D108449523 fbshipit-source-id: d53a81595a2f19ffcb79a5ab45d6ab4b7142008d

Split schema population out of `schematizer` Summary: `schematizer` uses unergonomic t_const_value population because it was originally shared with a sema mutator that injected a constant with the schema, however that integration has since been deleted and population only happens in t_ast_generator where generated Schema types are available. Move the existing `t_const_value` schema construction into generator-side `schema_populator` files so `schematizer` only owns the base identifier and schema const naming utilities needed below the `thrift2ast` cut line. Update BUCK and CMake build config along with the affected schematizer test dependency. Reviewed By: sadroeck Differential Revision: D107453925 fbshipit-source-id: c5f260791e2384a7fdb355e85308189e7f6c31c0

Compare Thrift structs in one traversal Summary: Optimize `op::compare` for generated Thrift ordering by making compare the primitive operation for structured values. `CompareThreeWay` walks struct, exception, union, list, set, and map values directly, and `LessThan` delegates to whether that result is `std::partial_ordering::less`, so struct comparison no longer synthesizes ordering by first evaluating equality and then less-than over the same fields. The adapter path keeps the existing `compareThreeWay` customization point as the top precedence hook and only falls back to `equal`/`less` synthesis when no compare hook is available. The default comparison implementation now uses `<=>` when available, otherwise it falls back to `std::equal_to` and `std::less`. Add `TestsetListListString` and `TestsetMapStringSetI64` cases to `thrift/test/benchmarks:compare` to measure public `op::compare` against the old equal-then-less baseline over generated `testset` structs with large nested containers and late differences. Final benchmark run at the rebased stack tip: | Benchmark | Equal-then-less baseline | `op::compare` | Relative | |---|---:|---:|---:| | `TestsetListListString` | `426.69us` | `233.66us` | `182.61%` | | `TestsetMapStringSetI64` | `1.06ms` | `637.02us` | `166.33%` | Reviewed By: hchokshi Differential Revision: D108952049 fbshipit-source-id: b4c3519ca8c388917abba17298f17b3591aa9b12

Emit object-key containers from compiler Summary: Lift the Hack generator restriction on non-arraykey map keys and set elements by emitting `\ThriftMap<K, V>` and `\ThriftSet<E>` for struct/union keys, bool/float keys, and adapted keys or set elements. The generated type specs mark these fields with `'format' => 'object_key'`, shape methods convert through `toShape()` / `fromShape()` and `toVec()` / `fromVec()`, and JSON/from-map paths continue to skip object-key fields that cannot be represented there. `edward-io.Wrapper`-affected object-key map keys and set elements are still rejected because they participate in serialized-key identity, while wrapped object-key map values are supported by wrapping/unwrapping values during generated shape and thrift-only conversions. Regenerated compiler fixtures include the dedicated `hack-object-key-containers` fixture for struct keys/elements, primitive object keys, recursive union keys, adapted typedef keys/elements, adapted map values, and wrapped map values. Reviewed By: a-lafrance Differential Revision: D108350999 fbshipit-source-id: 53b5242745f9eea9dabdc0062d150d0583a0e94a

Store object maps by serialized keys Summary: Store `ThriftMap<TKey, TValue>` entries by canonical serialized thrift key bytes. `set`, `get`, `at`, `contains`, and `remove` become normal byte-keyed dict operations; iterator/shape/key APIs deserialize fresh semantic keys so callers never see serialized bytes or mutate the stored key identity, while values remain the stored values. Shape construction rejects duplicate serialized keys instead of silently overwriting them. Traversal/conversion APIs intentionally randomize output order to avoid dependencies on the map implementation. This randomization is repeated for each instance of iteration (same as Go). The safety tradeoff is that keys are serialized on writes/lookups and deep-copied by deserializing on traversal; that costs CPU/allocation versus retaining object keys directly, but prevents later mutation of mutable thrift values from changing map identity. Reviewed By: a-lafrance Differential Revision: D108350997 fbshipit-source-id: 51978f41ff363c7d18f66729be4d49560a5d3d04

Populate schema definitions as generated C++ structs Summary: Rewrite `schema_populator` to construct generated `schema.thrift` C++ types directly instead of building `t_const_value` maps for `t_ast_generator` to hydrate. Keep source const/default interning on `t_const_value` (until the next stacked diff), but remove the schema-definition hydration path and update protocol-value tests to assert generated `protocol.Value` objects. Reviewed By: hchokshi Differential Revision: D107453926 fbshipit-source-id: 358275a96e50fd196fee9693ed8b56b5b8022440

Store object sets by serialized keys Summary: Store `ThriftSet<TElem>` entries as a `keyset<string>` of canonical serialized thrift element bytes. Membership uses serialized-byte membership, removal drops the serialized entry, and iterator/`toVec()` APIs deserialize fresh semantic elements rather than exposing serialized bytes. `fromVec()` rejects duplicate serialized elements instead of silently deduping them, and traversal/conversion APIs intentionally randomize output order to avoid dependencies on serialized-byte ordering. The safety tradeoff is that elements are serialized on add/lookup and deep-copied by deserializing on traversal; that costs CPU/allocation versus retaining element objects directly, but prevents later mutation of mutable thrift values from changing set identity. Reviewed By: a-lafrance Differential Revision: D108351002 fbshipit-source-id: d42a502def454912251a2302e8ab9ec387b14f41

Intern schema values without temporary `t_const_value` Summary: Move schema value interning above the cut line by making `schema_populator` own an `intern_func` that accepts generated `protocol::Value` directly. Convert field defaults, const values, structured annotation double-writes, and source-info strings to `protocol::Value` without cloning or constructing new `t_const_value`s, while keeping `schematizer` free of generated schema/protocol types. Reviewed By: hchokshi Differential Revision: D107455623 fbshipit-source-id: 52a455e18a372fe5e6a2d69901261f29a5c8b9c1

Test object-key containers through codegen Summary: Enable shape generation for `ProtocolTest.thrift`, regenerate the Meerkat-managed Hack output with the locally built thrift compiler, and add generated coverage for object-key map/set fields across struct, bool, float, adapted, and recursive union key paths. `ThriftObjectKeyContainerIntegrationTest` verifies generated fields are backed by `ThriftMap` and `ThriftSet`, round-trip through compact/binary serializers, expose semantic shape values through `__toShape()`, and reject duplicate object-key entries through `__fromShape()`. Reviewed By: vitaut Differential Revision: D108351001 fbshipit-source-id: 2fe91177033e689f2c6736d6bd697dfaa2ef89f1

Extract reusable Thrift parser core Summary: Move the recursive-descent parser implementation behind a templated internal `parser_core`, keep the compiler-facing `parse(lexer&, parser_actions&, diagnostics_engine&)` API as an adapter over `parser_actions`, and leave regular parser behavior unchanged while making the grammar reusable by concrete consumers. Reviewed By: hchokshi Differential Revision: D109361636 fbshipit-source-id: 22cf6ae6707f3af09eb282fe027d02f98ae4bf72

Avoid over-wrapping inline formatter output Summary: Keep formatter decisions source-shape aware when doing so does not hide trivia. This preserves source-inline `throws` clauses and structured annotation bodies, keeps source-block-formatted values and generic types block-formatted, and preserves explicit `optional`/`required` tokens even when semantic parsing normalizes them away. The formatter still falls back to multiline output whenever comments would be hidden. Reviewed By: vitaut Differential Revision: D110337471 fbshipit-source-id: 18087446ef669e4f73b162126d262e18c015df59

Modernize comparisons for `FieldRef` Summary: Replace free comparison operators in `FieldRef.h` with member `operator==` and `operator<=>` implementations across field ref wrappers. Preserve optional-style unset ordering with explicit comparison category deduction for mixed `std::strong_ordering` and value comparison results. This leverages C++20 operator synthesis and reversal to reduce duplication. Reviewed By: praihan, vitaut Differential Revision: D107275861 fbshipit-source-id: 77f1dc30c191bffc581a74b6d534e3591871e79e

Preserve content after trailing line comments Summary: Make the formatter inline continuation spacing consult trailing line comments generically instead of special-casing fields. When a printed token owns a //-style trailing comment, following inline content moves to the next line so fields, typedefs, consts, includes, namespaces, services, enum values, annotations, and function signatures do not get swallowed into the comment text. Reviewed By: hchokshi Differential Revision: D110081313 fbshipit-source-id: 075b493b3266986309b5bdd4c1097d9280ba1b9f

Support SimpleJSON object-key maps Summary: Encode object-key `ThriftMap` fields as SimpleJSON list-of-pairs instead of JSON object entries, since struct, bool, and other non-string object keys cannot be represented as JSON object keys. Teach the SimpleJSON protocol read path and the `JSONThriftSerializer` fast path to consume the same list-of-pairs representation back into `ThriftMap`, while leaving non-SimpleJSON protocols on the existing map encoding. Generated `readFromJson()` now delegates structs that contain object-key containers to `TSimplePHPObjectProtocol`, so the generated JSON reader uses the same object-key handling instead of silently dropping those fields. Reviewed By: a-lafrance Differential Revision: D109358334 fbshipit-source-id: 51ae7b40fdda9074440599c9d1252a8c4d6a46ad

Handle recursive sealed structs Summary: Guard `t_structured::is_sealed()` against recursive structured-type cycles so `thrift.Sealed` structs can contain containers of themselves without recursing until stack overflow. Add compiler coverage for a recursive sealed struct through `list`, `map`, and `set` fields. Reviewed By: ahilger Differential Revision: D110243801 fbshipit-source-id: dc324d441f4a40d32a5b5e5bac033945db69f067

Generate operator<=> for Thrift structured types Summary: Change Thrift C++ codegen to assume C++20 three-way comparison is available: generated default comparable/orderable structs, unions, and exceptions now unconditionally include `<compare>` and emit `std::partial_ordering operator<=>`. Remove the old `__cpp_impl_three_way_comparison` gate and default-path pre-C++20 `operator<`, while preserving explicitly requested legacy `operator<` declarations. The generated operator delegates to `apache::thrift::op::detail::CompareThreeWay`, matching `op::compare` semantics, including partial ordering for numeric tags and unordered results for NaN. Reviewed By: sadroeck Differential Revision: D107316879 fbshipit-source-id: 390a2583f460e4d1d1b30a86cc131cfe4e407162

Expose Thrift parser trivia to concrete consumers Summary: Add opt-in lexer trivia capture and parser token recording for concrete parser consumers. This keeps the compiler parser path unchanged while giving formatter actions access to comments/newlines without adding formatter state to the semantic AST. Reviewed By: hchokshi Differential Revision: D109361631 fbshipit-source-id: 1c1aa50c3cb1c9729fbd0e84a97f1c0f4020d35a

Route object-key structs through Hack serializers Summary: Emit `IThriftStructWithObjectKeyContainers` on generated structs with direct or nested object-key containers so the runtime can route those structs through the Hack protocol serializers with an O(1) marker-interface check. The native HHVM serializer paths still assume arraykey-backed map/set shapes, while the Hack protocol path can read and write `ThriftMap` and `ThriftSet` fields correctly. Reviewed By: a-lafrance, prakashgayasen Differential Revision: D108350996 fbshipit-source-id: fc61022bbc0f2d1a6e786885e23b46de621ecb52