Skip to content

Handle cyclic protobuf message references#166

Open
SOG-web wants to merge 1 commit into
Arwalk:zig-masterfrom
SOG-web:zig-master
Open

Handle cyclic protobuf message references#166
SOG-web wants to merge 1 commit into
Arwalk:zig-masterfrom
SOG-web:zig-master

Conversation

@SOG-web

@SOG-web SOG-web commented Mar 29, 2026

Copy link
Copy Markdown

This pull request introduces several improvements and bug fixes to the Zig Protobuf code generator and runtime. The most significant changes enhance support for self-referential message types, improve memory management for repeated fields, and refactor type handling for clarity and correctness. These updates make the code generator more robust when dealing with complex Protobuf schemas and ensure safer deallocation of generated structures.

Improvements to self-referential and repeated message handling:

  • Enhanced the detection and handling of self-referential message types in the code generator, ensuring that fields referencing their own message type are correctly recognized and pointerized as needed. This includes refactoring the logic for determining when a field needs to be a pointer and normalizing type names for consistency. [1] [2] [3] [4] [5]
  • Improved the initialization and decoding of repeated submessage fields to handle pointer types correctly, ensuring that messages are allocated, initialized, and appended to result lists in a memory-safe way.

Memory management improvements:

  • Updated the deinitialization logic for repeated fields to handle lists of pointers to structs, not just slices of bytes, ensuring proper destruction and deallocation of complex repeated fields. [1] [2]

Code organization and readability:

  • Refactored import ordering and grouping in several files for better readability and maintainability. [1] [2] [3]

These changes collectively improve the reliability and maintainability of both the code generator and the runtime library when working with advanced Protobuf features.

@menduz menduz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! thanks for sending this. Would you please include test cases? The logic is sound, but since the code is not the most straight-forward possible I'd like to prevent accidental regressions.

Comment thread src/wire.zig
Comment on lines +460 to +461
const result_ti = comptime @typeInfo(Result);
if (comptime result_ti == .pointer) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expression is always comptime, in both lines

@Arwalk

Arwalk commented Mar 29, 2026

Copy link
Copy Markdown
Owner

Aren't self-referential messages already handled? Do you have reproductible test cases showing where it would be failing?

@SOG-web

SOG-web commented Apr 1, 2026

Copy link
Copy Markdown
Author

Aren't self-referential messages already handled? Do you have reproductible test cases showing where it would be failing?

I tried using it on https://github.com/SOG-web/libpg_query/tree/main, but the generated was incomplete

@SOG-web

SOG-web commented Apr 1, 2026

Copy link
Copy Markdown
Author

Aren't self-referential messages already handled? Do you have reproductible test cases showing where it would be failing?

 const protobuf_dep = b.dependency("protobuf", .{
        .target = target,
        .optimize = optimize,
    });
    const protobuf_mod = protobuf_dep.module("protobuf");
     const pg_query_proto_step = protobuf.RunProtocStep.create(protobuf_dep.builder, target, .{
        .destination_directory = b.path("pg_query/src/generated"),
        .source_files = &.{b.path("libpg_query/protobuf/pg_query.proto")},
        .include_directories = &.{b.path("libpg_query/protobuf")},
    });
    const gen_proto = b.step("gen-proto", "Generate Zig bindings from pg_query.proto");
    gen_proto.dependOn(&pg_query_proto_step.step);
    ```

@menduz

menduz commented Apr 1, 2026

Copy link
Copy Markdown
Collaborator

Aren't self-referential messages already handled? Do you have reproductible test cases showing where it would be failing?

I tried using it on https://github.com/SOG-web/libpg_query/tree/main, but the generated was incomplete

Since the file is huge, it would be nice to extract the missing part and create a persistent isolated test in this pr

@Arwalk

Arwalk commented May 12, 2026

Copy link
Copy Markdown
Owner

@SOG-web if you could provide a test reproducing the problem, that would be nice. Thank you in advance.

@SOG-web

SOG-web commented May 12, 2026

Copy link
Copy Markdown
Author

I would try create a small sample soon, am currently not on the project I got the error

Sorry for the delay

@Arwalk

Arwalk commented May 12, 2026

Copy link
Copy Markdown
Owner

I would try create a small sample soon, am currently not on the project I got the error

Sorry for the delay

It's fine, we all have our lives and things to do :)

Thank you for your contributions.

@menduz

menduz commented May 19, 2026

Copy link
Copy Markdown
Collaborator

Implemented in #171

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants