You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@thrift.apache.org by "MPDR200011 (via GitHub)" <gi...@apache.org> on 2026/08/17 20:54:15 UTC
[PR] THRIFT-6152: Add Zig binding [thrift]
MPDR200011 opened a new pull request, #3730:
URL: https://github.com/apache/thrift/pull/3730
Zig code generator + runtime library with minimal feature set from the [new language guide.](https://thrift.apache.org/docs/HowToNewLanguage.html)
The code generator started as a copy paste of the rust one that I slowly chipped away at, so it is possible that some rust references are still there.
Cross language tests are implemented, along with some language specific tests that test the integration between the compiler and the library.
No CI jobs implemented yet.
- [x] Did you create an [Apache Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket? ([Request account here](https://selfserve.apache.org/jira-account.html), not required for trivial changes)
- [x] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
- [x] Did you squash your changes to a single commit? (not required, but preferred)
- [x] Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
- [x] If your change does not involve any code, include `[skip ci]` anywhere in the commit message to free up build resources.
<!--
The Contributing Guide at:
https://github.com/apache/thrift/blob/master/CONTRIBUTING.md
has more details and tips for committing properly.
-->
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6152: Add Zig binding [thrift]
Posted by "MPDR200011 (via GitHub)" <gi...@apache.org>.
MPDR200011 commented on code in PR #3730:
URL: https://github.com/apache/thrift/pull/3730#discussion_r3808023350
##########
tutorial/zig/build.zig.zon:
##########
@@ -0,0 +1,43 @@
+.{
+ // This is the default name used by packages depending on this one. For
+ // example, when a user runs `zig fetch --save <url>`, this field is used
+ // as the key in the `dependencies` table. Although the user can choose a
+ // different name, most users will stick with this provided value.
+ //
+ // It is redundant to include "zig" in this name because it is already
+ // within the Zig package namespace.
+ .name = .zig,
+ // This is a [Semantic Version](https://semver.org/).
+ // In a future version of Zig it will be used for package deduplication.
+ .version = "0.0.0",
Review Comment:
I see, I'll get that done while I setup CI for it
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6152: Add Zig binding [thrift]
Posted by "MPDR200011 (via GitHub)" <gi...@apache.org>.
MPDR200011 commented on code in PR #3730:
URL: https://github.com/apache/thrift/pull/3730#discussion_r3815756280
##########
lib/zig/src/lib/result.zig:
##########
@@ -0,0 +1,291 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+const std = @import("std");
+const protocol = @import("../protocol/protocol.zig");
+const app_exception = @import("application_exception.zig");
+const types = @import("types.zig");
+
+const TProtocol = protocol.TProtocol;
+const Allocator = std.mem.Allocator;
+const TApplicationException = app_exception.TApplicationException;
+
+pub fn ServiceCallResult(comptime ResultType: type, comptime ErrorType: type, comptime name: []const u8) type {
+ return union(enum) {
+ result: ResultType,
+ err: ErrorType,
+
+ pub fn success(val: ResultType) @This() {
+ return .{ .result = val };
+ }
+
+ pub fn fail(val: ErrorType) @This() {
+ return .{ .err = val };
+ }
+
+ pub fn deinit(self: *@This()) void {
+ return switch (self.*) {
+ .result => |*res| {
+ switch (@typeInfo(ResultType)) {
+ .void, .@"enum", .int, .float, .bool => return,
+ else => {},
+ }
+ if (ResultType == types.UUID) {
+ return;
+ }
+ res.deinit();
+ },
+ .err => |*e| {
+ if (ErrorType == void) {
+ return;
+ }
+ e.deinit();
+ },
+ };
+ }
+
+ pub fn get(self: *@This()) !ResultType {
+ switch (self.*) {
+ .result => |res| {
+ return res.result_value;
+ },
+ .err => return error.ErrorResult,
+ }
+ }
+
+ pub fn unwrapError(self: *@This()) ErrorType {
+ switch (self.*) {
+ .err => |err| {
+ return err;
+ },
+ .result => unreachable,
+ }
+ }
+
+ pub fn readFromProtocol(allocator: Allocator, iProt: *TProtocol) !@This() {
+ var structIdent = try iProt.readStructBegin(allocator);
+ defer structIdent.deinit();
+
+ var res: ?@This() = null;
+ while (true) {
+ var fieldId = try iProt.readFieldBegin(allocator);
+ defer fieldId.deinit();
+
+ if (fieldId.fieldType == .Stop) {
+ break;
+ }
+
+ switch (fieldId.id.?) {
+ 0 => {
+ if ((fieldId.fieldType == protocol.typeToTType(ResultType)) or
+ // String and Binary share the same in wire value,
+ // I've checked multiple implementations and those
+ // check if TType == String for Binary fields. I
+ // guess that's fine since they share the same in
+ // wire layout (at least in binary and compact
+ // protocols)
+ (fieldId.fieldType == .String and ResultType == types.BinaryBytes))
+ {
+ res = .{
+ .result = try protocol.readFromProtocol(ResultType, allocator, iProt),
+ };
+ } else {
+ try iProt.skip(allocator, fieldId.fieldType);
+ }
+ },
+ else => {
+ res = e: {
+ if (ErrorType == void) {
+ try iProt.skip(allocator, fieldId.fieldType);
+ break :e .{ .err = {} };
+ }
+ break :e .{
+ .err = try ErrorType.readFromProtocol(allocator, fieldId.id.?, iProt),
+ };
+ };
+ },
+ }
+ try iProt.readFieldEnd();
+ }
+ try iProt.readStructEnd();
+
+ if (res) |r| {
+ return r;
+ } else {
+ if (ResultType == void) {
+ return .success({});
+ }
+ return error.EmptyResultStruct;
+ }
+ }
+
+ pub fn writeToProtocol(self: *@This(), oProt: *TProtocol) !void {
+ try oProt.writeStructBegin(.{ .allocator = undefined, .name = name });
+ write: switch (self.*) {
+ .result => |res| {
+ if (ResultType == void) {
+ break :write;
+ }
+ try oProt.writeFieldBegin(.{ .allocator = undefined, .id = 0, .name = "success", .fieldType = protocol.typeToTType(ResultType) });
+ try protocol.writeToProtocol(ResultType, res, oProt);
+ try oProt.writeFieldEnd();
+ },
+ .err => |*e| {
+ if (ErrorType == void) {
+ return;
+ } else {
+ try e.writeToProtocol(oProt);
+ }
+ },
+ }
+ try oProt.writeFieldStop();
+ try oProt.writeStructEnd();
+ }
+ };
+}
+
+// DEPRECATED
Review Comment:
my bad, forgot to clean that up
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6152: Add Zig binding [thrift]
Posted by "MPDR200011 (via GitHub)" <gi...@apache.org>.
MPDR200011 commented on code in PR #3730:
URL: https://github.com/apache/thrift/pull/3730#discussion_r3807898737
##########
tutorial/zig/src/tutorial_client.zig:
##########
@@ -0,0 +1,83 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+const std = @import("std");
+const thrift = @import("thrift");
+const tutorial = @import("tutorial");
+
+const Allocator = std.mem.Allocator;
+
+pub fn main(init: std.process.Init) !void {
+ var debug_allocator: std.heap.DebugAllocator(.{}) = .init;
+ defer switch (debug_allocator.deinit()) {
+ .leak => std.debug.print("UNFREE'D MEMORY DETECTED!!!! CHECK YOUR ALLOCS!!! >:(\n", .{}),
+ .ok => std.debug.print("No memory leaks detected :)\n", .{}),
+ };
+ const gpa = debug_allocator.allocator();
+ var config = thrift.TConfiguration.default;
+
+ var socket = try thrift.TSocket.initFromAddress(gpa, init.io, .{ .ip4 = .{ .bytes = [4]u8{0 ,0 ,0 ,0}, .port = 9069}});
Review Comment:
> does this even work on client side?
hm, surprisingly, it does on my machine. I'll change that to `127.0.0.1` to avoid confusion, though.
> so `0, 0, 0, 0` over `0 ,0 ,0 ,0`
good catch, looks like uncoordinated hands on my part
##########
tutorial/zig/build.zig.zon:
##########
@@ -0,0 +1,43 @@
+.{
+ // This is the default name used by packages depending on this one. For
+ // example, when a user runs `zig fetch --save <url>`, this field is used
+ // as the key in the `dependencies` table. Although the user can choose a
+ // different name, most users will stick with this provided value.
+ //
+ // It is redundant to include "zig" in this name because it is already
+ // within the Zig package namespace.
+ .name = .zig,
+ // This is a [Semantic Version](https://semver.org/).
+ // In a future version of Zig it will be used for package deduplication.
+ .version = "0.0.0",
Review Comment:
I assume you mean the library version and not the tutorial? Is that a convention for other libraries?
It would mean having to manually update this every time a new release of thrift comes out, which sounds like an easily missed chore.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6152: Add Zig binding [thrift]
Posted by "Jens-G (via GitHub)" <gi...@apache.org>.
Jens-G commented on PR #3730:
URL: https://github.com/apache/thrift/pull/3730#issuecomment-5417002385
### Code review
Found 9 issues:
1. `$(MKDIR)` is not a variable this build defines — `configure.ac:94` uses `AC_PROG_MKDIR_P`, which substitutes `$(MKDIR_P)`. It expands to nothing, so the recipe runs as `-p gen-zig`, `gen-zig/` is never created and the following `thrift -out gen-zig` fails. These two lines are the only bare `$(MKDIR)` in the repo; `tutorial/zig/Makefile.am` uses plain `mkdir -p` and is unaffected. This is why `lib-zig` is red, and because `cross-test` lists `lib-zig` in `needs:` it is *skipped* rather than failed — so the Zig cross-language tests have not run against this branch. Same fix in `test/zig/Makefile.am#L22-L24`.
https://github.com/apache/thrift/blob/36eb758bfc2ce646361470eaac043453074bfde1/lib/zig/tests/Makefile.am#L26-L28
2. Compact protocol cannot write `bool` inside a container: `tTypeToByte` maps `.Bool` to an error alongside `.Stop`/`.Void`, so any `list<bool>`, `set<bool>` or bool-keyed/valued map fails with `NoTypeValueForType`. The read path already accepts it (`byteToTType` maps `1, 2 => .Bool`) and `binary_protocol.zig` maps `.Bool => 2`, so this is compact-only. Java uses `ttypeToCompactType[TType.BOOL] = Types.BOOLEAN_TRUE` and Go `BOOL: COMPACT_BOOLEAN_TRUE`. `ThriftTest.thrift` has no bool containers and the conformance suite only uses `.I32` elements, so the cross-tests would not surface it.
https://github.com/apache/thrift/blob/36eb758bfc2ce646361470eaac043453074bfde1/lib/zig/src/protocol/compact_protocol.zig#L53-L57
3. `readVarint32` narrows with a checked `@intCast`, which aborts instead of returning a protocol error. `readVarint` bounds the byte count but not the magnitude, so five bytes decode up to 2^35-1 — `80 80 80 80 08` gives 2^31, one past `i32` max — and this runs before `checkReadLength`/`checkContainerSize`. It also breaks interop: Java's `writeVarint32` uses `n >>>= 7`, so a negative seqid from any other binding encodes into [2^31, 2^32) and aborts a Zig server on parse. `writeVarint32` at `L133-L135` has the mirror problem on the write side. Relatedly, `shift` is a `u6` incremented before the `shift >= maxBits` guard, so for `i64` it reaches 63, passes the guard, then overflows on the next `+= 7` — the guard is unreachable for `i64`.
https://github.com/apache/thrift/blob/36eb758bfc2ce646361470eaac043453074bfde1/lib/zig/src/protocol/compact_protocol.zig#L136-L140
https://github.com/apache/thrift/blob/36eb758bfc2ce646361470eaac043453074bfde1/lib/zig/src/protocol/compact_protocol.zig#L166-L177
4. The `TTransport` vtable's `destroy` shim does not dispatch to the concrete type. `deinit` just above it does (`ptr_info.pointer.child.deinit(self)`), as do `open`/`close`/`isOpen`/`reader`/`writer`/`flush`, but `destroy` calls `a.destroy(self)` directly. That makes `TFramedTransport.destroy()`'s cascade (`self.underlying.destroy(a)`) unreachable, so the wrapped transport is never freed — contradicting the comment at `framed_transport.zig#L41` that this transport "owns the underlying transport interface, i.e. manages it's lifecycle". `TSimpleServer` destroys through the interface per accepted connection.
https://github.com/apache/thrift/blob/36eb758bfc2ce646361470eaac043453074bfde1/lib/zig/src/transport/interface.zig#L43-L52
5. Reserved-word escaping is applied to declarations but not to references, so the two disagree. `zig_struct_nameb`/`zig_field_name` route through `zig_safe_name`, but `to_zig_type` returns `zig_namespace(ttype) + ttype->get_name()` unescaped for enums and structs — a struct named `error` is declared `error_` and referenced as `error`. `service_call_client_function_name`/`service_call_handler_function_name` (`L3360-L3366`) and the enum name and members in `generate_enum` (`L807`, `L816`) are likewise unescaped. `ZIG_RESERVED_WORDS` includes `error`, `test`, `union`, `return`, `continue`. There is no keyword fixture in the PR, so nothing exercises it — same shape as THRIFT-6114/6115/6116 in the Python generator.
https://github.com/apache/thrift/blob/36eb758bfc2ce646361470eaac043453074bfde1/compiler/cpp/src/thrift/generate/t_zig_generator.cc#L3187-L3192
6. Wire-declared container counts and string lengths are never checked against `TConfiguration.maxMessageSize` — `checkContainerSize` and `checkReadLength` reject negatives only and take no configuration. `TList.readFromProtocol` then reserves capacity straight from that count, whereas `map.zig` and `set.zig` fill incrementally, so `list.zig` is the outlier. The framed transport bounds frame bytes, not the count declared inside them. Since this binding ships finite defaults (`maxMessageSize` 100 MiB), `doc/thrift-threat-model.md` treats the gap as in scope rather than property-disclaimed; Go's equivalent is `checkContainerSizeForProtocol`.
https://github.com/apache/thrift/blob/36eb758bfc2ce646361470eaac043453074bfde1/lib/zig/src/protocol/utils.zig#L48-L54
https://github.com/apache/thrift/blob/36eb758bfc2ce646361470eaac043453074bfde1/lib/zig/src/collections/list.zig#L119-L121
7. `TLoggingProtocolFactory.getProtocol` dupes `"logging"` and passes it to `TLoggingProtocol.init`, which dupes it again; `deinit` frees only the second copy, so the first leaks on every call. `test/zig/src/test_server.zig` installs this factory as the server protocol factory, so it leaks per connection.
https://github.com/apache/thrift/blob/36eb758bfc2ce646361470eaac043453074bfde1/lib/zig/src/protocol/logging_protocol.zig#L322-L327
8. Five new files are missing the ASF license header that the rest of the new `.zig` files carry: `lib/zig/src/collections/utils.zig`, `lib/zig/src/internal/mod.zig`, `lib/zig/src/internal/protocol.zig`, `lib/zig/src/internal/struct_utilities.zig` and `lib/zig/src/lib/types.zig`. `lib/zig/src/collections/list.zig` in the same directory has one, and there is no RAT check in CI that would catch the omission.
https://github.com/apache/thrift/blob/36eb758bfc2ce646361470eaac043453074bfde1/lib/zig/src/collections/utils.zig#L1-L5
9. Dead scaffolding, and none of it compiles. `ClientResult` is marked `// DEPRECATED` and is never instantiated — the generator hand-rolls its own `...ClientResult` type per method in `render_client_result_union` — yet both it and `ServiceCallResult` are exported unqualified from `root.zig#L72-L73`, and `README.md:238` points users at the `*ClientResult` name. Because Zig only analyses referenced functions, this code is never type-checked, and all of it is broken: `ServiceCallResult.get()` (`result.zig#L61-L68`) returns `res.result_value` where `res` is already the value and no such field exists; `TApplicationException.eql()` (`application_exception.zig#L74-L81`) declares `u64` but returns `bool` and falls off the end on the equal path; `Map.format()` (`map.zig#L140-L142`) prints the key twice instead of `key: value`. For a new binding this seems worth deleting rather than shipping.
https://github.com/apache/thrift/blob/36eb758bfc2ce646361470eaac043453074bfde1/lib/zig/src/lib/result.zig#L160-L164
🤖 Generated with [Claude Code](https://claude.ai/code)
<sub>- If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub>
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6152: Add Zig binding [thrift]
Posted by "fishy (via GitHub)" <gi...@apache.org>.
fishy commented on code in PR #3730:
URL: https://github.com/apache/thrift/pull/3730#discussion_r3799087101
##########
tutorial/zig/build.zig.zon:
##########
@@ -0,0 +1,43 @@
+.{
+ // This is the default name used by packages depending on this one. For
+ // example, when a user runs `zig fetch --save <url>`, this field is used
+ // as the key in the `dependencies` table. Although the user can choose a
+ // different name, most users will stick with this provided value.
+ //
+ // It is redundant to include "zig" in this name because it is already
+ // within the Zig package namespace.
+ .name = .zig,
+ // This is a [Semantic Version](https://semver.org/).
+ // In a future version of Zig it will be used for package deduplication.
+ .version = "0.0.0",
Review Comment:
this probably should match the upcoming thrift release version (0.25.0).
##########
tutorial/zig/src/tutorial_client.zig:
##########
@@ -0,0 +1,83 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+const std = @import("std");
+const thrift = @import("thrift");
+const tutorial = @import("tutorial");
+
+const Allocator = std.mem.Allocator;
+
+pub fn main(init: std.process.Init) !void {
+ var debug_allocator: std.heap.DebugAllocator(.{}) = .init;
+ defer switch (debug_allocator.deinit()) {
+ .leak => std.debug.print("UNFREE'D MEMORY DETECTED!!!! CHECK YOUR ALLOCS!!! >:(\n", .{}),
+ .ok => std.debug.print("No memory leaks detected :)\n", .{}),
+ };
+ const gpa = debug_allocator.allocator();
+ var config = thrift.TConfiguration.default;
+
+ var socket = try thrift.TSocket.initFromAddress(gpa, init.io, .{ .ip4 = .{ .bytes = [4]u8{0 ,0 ,0 ,0}, .port = 9069}});
Review Comment:
do we actually use 0.0.0.0 on the client side? we use it on the server side to listen on all available IPs, but on client side we usually use 127.0.0.1 instead? does this even work on client side?
also I'm not sure if this is some special zig formatting convention, but don't you usually put space _after_ comma instead of before? so `0, 0, 0, 0` over `0 ,0 ,0 ,0`.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6152: Add Zig binding [thrift]
Posted by "fishy (via GitHub)" <gi...@apache.org>.
fishy commented on code in PR #3730:
URL: https://github.com/apache/thrift/pull/3730#discussion_r3808684659
##########
lib/zig/src/lib/result.zig:
##########
@@ -0,0 +1,291 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+const std = @import("std");
+const protocol = @import("../protocol/protocol.zig");
+const app_exception = @import("application_exception.zig");
+const types = @import("types.zig");
+
+const TProtocol = protocol.TProtocol;
+const Allocator = std.mem.Allocator;
+const TApplicationException = app_exception.TApplicationException;
+
+pub fn ServiceCallResult(comptime ResultType: type, comptime ErrorType: type, comptime name: []const u8) type {
+ return union(enum) {
+ result: ResultType,
+ err: ErrorType,
+
+ pub fn success(val: ResultType) @This() {
+ return .{ .result = val };
+ }
+
+ pub fn fail(val: ErrorType) @This() {
+ return .{ .err = val };
+ }
+
+ pub fn deinit(self: *@This()) void {
+ return switch (self.*) {
+ .result => |*res| {
+ switch (@typeInfo(ResultType)) {
+ .void, .@"enum", .int, .float, .bool => return,
+ else => {},
+ }
+ if (ResultType == types.UUID) {
+ return;
+ }
+ res.deinit();
+ },
+ .err => |*e| {
+ if (ErrorType == void) {
+ return;
+ }
+ e.deinit();
+ },
+ };
+ }
+
+ pub fn get(self: *@This()) !ResultType {
+ switch (self.*) {
+ .result => |res| {
+ return res.result_value;
+ },
+ .err => return error.ErrorResult,
+ }
+ }
+
+ pub fn unwrapError(self: *@This()) ErrorType {
+ switch (self.*) {
+ .err => |err| {
+ return err;
+ },
+ .result => unreachable,
+ }
+ }
+
+ pub fn readFromProtocol(allocator: Allocator, iProt: *TProtocol) !@This() {
+ var structIdent = try iProt.readStructBegin(allocator);
+ defer structIdent.deinit();
+
+ var res: ?@This() = null;
+ while (true) {
+ var fieldId = try iProt.readFieldBegin(allocator);
+ defer fieldId.deinit();
+
+ if (fieldId.fieldType == .Stop) {
+ break;
+ }
+
+ switch (fieldId.id.?) {
+ 0 => {
+ if ((fieldId.fieldType == protocol.typeToTType(ResultType)) or
+ // String and Binary share the same in wire value,
+ // I've checked multiple implementations and those
+ // check if TType == String for Binary fields. I
+ // guess that's fine since they share the same in
+ // wire layout (at least in binary and compact
+ // protocols)
+ (fieldId.fieldType == .String and ResultType == types.BinaryBytes))
+ {
+ res = .{
+ .result = try protocol.readFromProtocol(ResultType, allocator, iProt),
+ };
+ } else {
+ try iProt.skip(allocator, fieldId.fieldType);
+ }
+ },
+ else => {
+ res = e: {
+ if (ErrorType == void) {
+ try iProt.skip(allocator, fieldId.fieldType);
+ break :e .{ .err = {} };
+ }
+ break :e .{
+ .err = try ErrorType.readFromProtocol(allocator, fieldId.id.?, iProt),
+ };
+ };
+ },
+ }
+ try iProt.readFieldEnd();
+ }
+ try iProt.readStructEnd();
+
+ if (res) |r| {
+ return r;
+ } else {
+ if (ResultType == void) {
+ return .success({});
+ }
+ return error.EmptyResultStruct;
+ }
+ }
+
+ pub fn writeToProtocol(self: *@This(), oProt: *TProtocol) !void {
+ try oProt.writeStructBegin(.{ .allocator = undefined, .name = name });
+ write: switch (self.*) {
+ .result => |res| {
+ if (ResultType == void) {
+ break :write;
+ }
+ try oProt.writeFieldBegin(.{ .allocator = undefined, .id = 0, .name = "success", .fieldType = protocol.typeToTType(ResultType) });
+ try protocol.writeToProtocol(ResultType, res, oProt);
+ try oProt.writeFieldEnd();
+ },
+ .err => |*e| {
+ if (ErrorType == void) {
+ return;
+ } else {
+ try e.writeToProtocol(oProt);
+ }
+ },
+ }
+ try oProt.writeFieldStop();
+ try oProt.writeStructEnd();
+ }
+ };
+}
+
+// DEPRECATED
Review Comment:
why do we add something that's already deprecated?
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6152: Add Zig binding [thrift]
Posted by "Jens-G (via GitHub)" <gi...@apache.org>.
Jens-G commented on PR #3730:
URL: https://github.com/apache/thrift/pull/3730#issuecomment-5648879125
Any news here?
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6152: Add Zig binding [thrift]
Posted by "Jens-G (via GitHub)" <gi...@apache.org>.
Jens-G commented on code in PR #3730:
URL: https://github.com/apache/thrift/pull/3730#discussion_r3837004281
##########
tutorial/zig/src/tutorial_client.zig:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+const std = @import("std");
+const thrift = @import("thrift");
+const tutorial = @import("tutorial");
+
+const Allocator = std.mem.Allocator;
+
+pub fn main(init: std.process.Init) !void {
+ var debug_allocator: std.heap.DebugAllocator(.{}) = .init;
+ defer switch (debug_allocator.deinit()) {
+ .leak => std.debug.print("UNFREE'D MEMORY DETECTED!!!! CHECK YOUR ALLOCS!!! >:(\n", .{}),
Review Comment:
I can't help but whenever I see this I get reminded of [this sentence](https://www.goodreads.com/quotes/639349-multiple-exclamation-marks-he-went-on-shaking-his-head-are). Aside from that, why are we yelling? Lastly, I am not sure if that sort of error popping up in a tutorial really is such a great idea at all - personal opinion of course.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6152: Add Zig binding [thrift]
Posted by "MPDR200011 (via GitHub)" <gi...@apache.org>.
MPDR200011 commented on PR #3730:
URL: https://github.com/apache/thrift/pull/3730#issuecomment-5334235361
Update: fixed the ip address on the tutorial client and did a round of `zig fmt` on the library.
Working on setting up a CI job
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6152: Add Zig binding [thrift]
Posted by "MPDR200011 (via GitHub)" <gi...@apache.org>.
MPDR200011 commented on PR #3730:
URL: https://github.com/apache/thrift/pull/3730#issuecomment-5334708571
- Added `style-local` to the tutorial and test libraries
- Created CI job for the library
I ended up also adding zig to the cross language tests, let me know if that's OK. I ran them against the go, nodejs, python and lua and all seems to pass. Was having issues building the C++ and Java ones so didn't add those to the pool yet. I'll try running them in one of the build docker containers soon.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6152: Add Zig binding [thrift]
Posted by "fishy (via GitHub)" <gi...@apache.org>.
fishy commented on code in PR #3730:
URL: https://github.com/apache/thrift/pull/3730#discussion_r3807921103
##########
tutorial/zig/build.zig.zon:
##########
@@ -0,0 +1,43 @@
+.{
+ // This is the default name used by packages depending on this one. For
+ // example, when a user runs `zig fetch --save <url>`, this field is used
+ // as the key in the `dependencies` table. Although the user can choose a
+ // different name, most users will stick with this provided value.
+ //
+ // It is redundant to include "zig" in this name because it is already
+ // within the Zig package namespace.
+ .name = .zig,
+ // This is a [Semantic Version](https://semver.org/).
+ // In a future version of Zig it will be used for package deduplication.
+ .version = "0.0.0",
Review Comment:
yes the library version. we bump library versions after every release. see b7b838d4508a79d37fe0bf8eb1065d0ebe4c30d9 for an example.
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org