Skip to content

Improved handling of ZST generic arguments - #1189

Open
fresheed wants to merge 32 commits into
mozilla:mainfrom
fresheed:fix_removed_generics
Open

fresheed wants to merge 32 commits into
mozilla:mainfrom
fresheed:fix_removed_generics

Conversation

@fresheed

@fresheed fresheed commented Oct 6, 2026 •

Copy link
Copy Markdown

Overview

This PR makes zero-sized types with alignment 1 (1-ZSTs) explicitly accounted for during the generics instantiation. It fixes a number of reported issues that previously either caused a panic, or produced ill-formed bindings. In case of C++ (where generics are not monomorphized), ill-formed instantiations are reported as warnings. It also leads to the change of the mangling scheme, which might affect users relying on the old one.

Most of the files affected by the PR are the new test cases and their expectation files. Among the existing test cases, only the expectations for opaque.rs changed, due to the new mangling.
The main logic is in monomorph.rs (the C++ check), ir/ty.rs and ir/generic_path.rs (ZST propagation), and ir/typedef.rs (ZST typedef instantiations). The other ir/*.rs files mostly just switch from Option<Type> to Result<Type, Zst>, and mangle.rs, cdecl.rs and library.rs have small related changes. tests/tests.rs adds the warning checks, which don't affect existing tests.

Notable changes

Propagation of ZSTs through load and monomorphization

Currently, 1-ZSTs are dropped upon load. Because of that, it might panic (when GenericParams::call receives fewer arguments than expected) or incorrectly match the generic arguments and their values (when ZST is followed by other arguments with default values).

To avoid this, ZSTs must be propagated all the way until the monomorphization. Thus, GenericArgument now has a new variant holding a value of Zst type. Such Zst value is a placeholder for any zero-sized type. All such types that are currently accounted for (i.e. dropped) are 1-aligned, and therefore Zst is an enum with a single variant Zst1. Keeping this specific variant might be useful for solving issues such as #1035 in the future when ZSTs with non-1 alignment are added.

Most of code in bindgen/ir/ related to loading or specialization that was returning Type / Option<Type> before now returns Result<Type, Zst>. Overall, handling of 1-ZSTs did not change: zero-sized struct fields and function arguments are dropped, and pointers to ZSTs are represented as void*. I have added one extra case: arrays of 1-ZSTs discovered upon specialization are also treated as 1-ZSTs; upon load, I preserve the existing behavior of returning an error immediately.

Change of mangling scheme

On the bindings side, the main visible change is that 1-ZST generic arguments are represented as 1Z placeholders. That is, Result<Foo, ()> is now mangled into Result_Foo__1Z rather than Result_Foo (see the changed expectations for opaque.rs test case - the only one affected). This solves problems arising from mangling into non-existent type (Option<()> into Option) or mangling two distinct types into the same one (Result<(), u32> and Result<u32, ()> into Result_u32).

1Z is chosen so that it cannot collide with an actual Rust identifier.

Detection and warning for specific ZST instantiations in C++

Generation of C bindings monomorphizes generic Rust types, emitting new data structures where ZST fields are simply dropped. With C++ however, there is no monomorphization, and generic Rust types are emitted as template types instantiated at use sites. This leads to potential problems where an instantiation with ZST creates void fields, arrays and function arguments.

Solving it in general is out of scope of this PR. Instead, I simply check whether there are potentially problematic instantiations (which requires running collect_monomorphs(), similar to the C case) and emit warnings for them. For that, I decided to keep track of the generic used to instantiate a given monomorph, and use it to check whether monomorphization caused any drops. This is implemented by have_shrunk function that essentially follows Type::specialize, i.e. detects the cases where Type::specialize altered the structure of input type; if Type::specialize changes, have_shrunk should be adapted accordingly.

This is a relatively cheap solution among the alternatives (see below).

New annotations for expected warnings

To test for the above warnings, I added two new annotations:

  • //! cbindgen:test-expect-no-warnings requires that the test case produces no warnings
  • cbindgen:test-expect-warning-<lang>=<text> requires that generating bindings for lang generates a warning containing text

These conditions are checked on captured stderr. Right now these annotations are only used in the new tests.

New tests

  • zst_valid.rs: ZST generic arguments that must not produce any warnings.
  • zst_issues.rs: the related issue examples that produce C++ warnings.
  • zst_generics.rs: similar to zst_valid.rs, but with examples that produce C++ warnings
  • zst_nested.rs: ZST arguments hidden in arrays and function types.
  • zst_typedefs.rs: C++ checks of generic typedef instantiations. It also has a commented-out example that would produce non-compilable bindings (see the question about ZST typedefs below)

Related issues

Closes #659, closes #99, closes #228.

Issue Example Upstream: C Upstream: C++ This PR: C This PR: C++
#228 (and similar example in #99) original: S<u32, ()> panic in GenericParams::call wrong number of template args correct generated, warning (S<uint32_t, void>)
#527 by @GoldsteinE: Either<(), u8> panic in GenericParams::call wrong number of template args correct layout; empty Left body (GCC extension) generated, warning (Either<void, uint8_t>)
#527 by @TheBlueMatt: CResultTempl<(), APIError> panic in GenericParams::call wrong number of template args correct correct, no warnings
#659 original: MyStruct<()> undefined type MyStruct MyStruct without template args correct generated, warning (MyStruct<void>)
#659 by @stevenengler: Foo<()> with PhantomData<T> undefined type Foo Foo without template args correct correct, no warnings

"Generated, warning" means the header compiles, but the warned-about type is ill-formed in C++ once instantiated (e.g. used by value).

Questions

Should more 1-ZSTs be added?

Upstream ty.rs only handles specific 1-ZSTs: () (as an empty tuple), PhantomData and PhantomPinned. I also made arrays of these to be treated as a 1-ZST (upon specialization). It seems useful to handle more types, in particular, an empty struct.

Alternative implementations of C++ checks

There are other potential solutions for this (extending Type::specialize to immediately report issues, or changing its return type to something like 'ir::Type with ZSTs' and converting it to ir::Type later), but they seem to require many more changes.

Handling of typedefs with ZST generics

Upstream doesn't account for ZST instantiations of generic typedefs specifically. Since ZSTs are dropped upon load, in C it leads to "unknown name" gcc errors or panic, and in C++ these typedefs are used without arguments which doesn't compile either. I'm not sure what is the expected solution for that in general.

In this PR, generic typedefs aliasing a ZST are not emitted at all - we display a warning instead; this is inspired by the upstream behavior mentioned below. For C++, generic typedefs instantiations are checked alongside structs etc. with have_shrunk. For C, "unknown name" errors still appear; however, the produced C bindings are at least syntactically correct and don't cause panics. There are also options of emitting opaque structs (similar to what upstream does upon loading of non-generic ZST typedefs), or emitting typedef void ... (but this contradicts the aforementioned upstream behavior).

AI disclosure

I used Claude Code w/ Opus 5.5 to help navigating the codebase, suggesting test cases and generating small pieces of code, and also for finding the related issues. All generated code is reviewed by me. I also used AI to iterate on this PR description.

@fresheed

Copy link
Copy Markdown
Author

@emilio would it be better to split this PR into few parts? For example:

  1. ZST generic propagation (to fix panics)
  2. change to name mangling
  3. C++ - specific checks

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

Labels

None yet

Projects

None yet

1 participant