Subject: Re: [PATCH v2 01/10] doc: define unambiguous type mappings across C and Rust

From: Phillip Wood <[email-masked]>
Date: Sun, 9 Nov 2025 14:14:11 +0000

On 06/11/2025 22:52, Ezekiel Newren wrote:
> On Thu, Nov 6, 2025 at 2:55 AM Phillip Wood <[email-masked]> wrote:
>> On 29/10/2025 22:19, Ezekiel Newren via GitGitGadget wrote:
>>> From: Ezekiel Newren <[email-masked]>
>>>
>>> Document other nuances with crossing the FFI boundary. Other language
>>> mappings may be added in the future.
>>
>> Thanks for adding this, I've left a few comments below. Overall I
>> thought it was very well written.
> 
> Thanks.
> 
> I felt it was necessary since C vs Rust types keep coming up over and
> over again. I'm flexible with the wording of this document. I was just
> trying to convey a firm and clear stance on what is and isn't proper
> in Git.

That will definitely be useful as we add more rust code. In the future 
we may want to add a summary of which types to use to 
Documentation/CodingGuidelines but that doesn't need to be done in this 
series.

>> I tried building an html version of
>> this but even after adding it to the list of TECH_DOCS in
>> Documentation/Makefile with
>>
>> diff --git a/Documentation/Makefile b/Documentation/Makefile
>> index 47208269a2e..2699f0b24af 100644
>> --- a/Documentation/Makefile
>> +++ b/Documentation/Makefile
>> @@ -143,6 +143,7 @@ TECH_DOCS += technical/shallow
>>    TECH_DOCS += technical/sparse-checkout
>>    TECH_DOCS += technical/sparse-index
>>    TECH_DOCS += technical/trivial-merge
>> +TECH_DOCS += technical/unambiguous-types
>>    TECH_DOCS += technical/unit-tests
>>    SP_ARTICLES += $(TECH_DOCS)
>>    SP_ARTICLES += technical/api-index
>>
>> it fails with
>>
>> $ make -C Documentation/ technical/unambiguous-types.html
>>                                         Merge branch
>> 'ps/object-source-loose' into seen
>> make: Entering directory '/home/phil/src/git/Documentation'
>>       GEN asciidoc.conf
>>       * new asciidoc flags
>>       ASCIIDOC technical/unambiguous-types.html
>> asciidoc: ERROR: unambiguous-types.adoc: line 139: undefined filter
>> attribute in command: source-highlight --gen-version -f xhtml -s
>> {language} {src_numbered?--line-number=' '} {src_tab?--tab={src_tab}}
>> {args=}
>> asciidoc: ERROR: unambiguous-types.adoc: line 162: undefined filter
>> attribute in command: source-highlight --gen-version -f xhtml -s
>> {language} {src_numbered?--line-number=' '} {src_tab?--tab={src_tab}}
>> {args=}
>> asciidoc: ERROR: unambiguous-types.adoc: line 177: undefined filter
>> attribute in command: source-highlight --gen-version -f xhtml -s
>> {language} {src_numbered?--line-number=' '} {src_tab?--tab={src_tab}}
>> {args=}
>> asciidoc: ERROR: unambiguous-types.adoc: line 187: undefined filter
>> attribute in command: source-highlight --gen-version -f xhtml -s
>> {language} {src_numbered?--line-number=' '} {src_tab?--tab={src_tab}}
>> {args=}
>> asciidoc: ERROR: unambiguous-types.adoc: line 199: undefined filter
>> attribute in command: source-highlight --gen-version -f xhtml -s
>> {language} {src_numbered?--line-number=' '} {src_tab?--tab={src_tab}}
>> {args=}
>> asciidoc: ERROR: unambiguous-types.adoc: line 213: undefined filter
>> attribute in command: source-highlight --gen-version -f xhtml -s
>> {language} {src_numbered?--line-number=' '} {src_tab?--tab={src_tab}}
>> {args=}
>> asciidoc: ERROR: unambiguous-types.adoc: line 224: undefined filter
>> attribute in command: source-highlight --gen-version -f xhtml -s
>> {language} {src_numbered?--line-number=' '} {src_tab?--tab={src_tab}}
>> {args=}
>> make: *** [Makefile:396: technical/unambiguous-types.html] Error 1
>> make: *** Deleting file 'technical/unambiguous-types.html'
>> make: Leaving directory '/home/phil/src/git/Documentation'
> 
> I've never created documentation for Git before, so this helps. I'll
> incorporate your suggestions.

We should also add this file to Documentation/technical/meson.build. It 
seems those errors above are due to some incompatibility between 
asciidoc and asciidoctor as I just tried running

     make -C Documentation/ USE_ASCIIDOCTOR=1 
technical/unambiguous-types.html

and it worked just fine. I'm afraid I don't know enough asciidoc to make 
any helpful suggestions on how to fix it.

>>> +== Character types
>>> +
>>> +This is where C and Rust don't have a clean one-to-one mapping. A C `char` is
>>> +an 8-bit type that is signless (neither signed nor unsigned)
>>
>> I found this a bit confusing. Isn't the signedness of "char"
>> implementation defined rather than it being "signless"
>>
>>> which causes
>>> +problems with e.g. `make DEVELOPER=1`.
>>
>> I'm not sure what this is referring to - maybe -Wsign-compare?
> 
> When I build Git with `make DEVELOPER=1` and I compare uint8_t with
> char it complains about a difference in signedness. When I compare
> int8_t with char it also complains about a difference in signedness.
> So it is implementation defined, but it's also neither signed nor
> unsigned according to DEVELOPER=1 since it complains either way.

Oh, I see - this is saying mixing "char" and "uint8_t" causes problems. 
I agree, perhaps we could expand this slightly to mention comparison 
with uint8_t to make it clearer.

>>> Rust's `char` type is an unsigned 32-bit
>>> +integer that is used to describe Unicode code points. Even though a C `char`
>>> +is the same width as `u8`, `char` should be converted to u8 where it is
>>> +describing bytes in memory.
>>
>> I'm dreading the point where we start sharing "struct strbuf" with rust
>> and have to change the "buf" member from "char*" to "uint8_t*". While it
>> is not used in the xdiff code it is ubiquitous everywhere else and there
>> are lots of places where be pass the "buf" member to functions expecting
>> a "char*".
>>
>>          git grep -E '(\.|->)buf\W'
>>
>> has over 4000 matches
> 
> This is why I started in Xdiff since its code is mostly isolated.

Good plan!

> I
> think that we might have to bite the bullet and deal with the ugly
> mapping of char on the C side and u8 on the Rust side when dealing
> with strbuf. Maybe as we translate more of C into Rust someone will
> have a better suggestion. I think my ivec type would be better since
> strbuf is almost a special case of my ivec type, but dealing with
> strbuf is outside the scope of this patch series.

Yes, hopefully it will become clearer what the least painful route 
forward is as we get more experience with rust <=> C iterop.

>>> +While you could specify `char` in the C code and `u8` in Rust code, it's not as
>>> +clear what the appropriate type is, but it would work across the FFI boundary.
>>> +However the bigger problem comes from code generation tools like cbindgen and
>>> +bindgen. When cbindgen see u8 in Rust it will generate uint8_t on the C side
>>> +which will cause differ in signedness warnings/errors. Similarly if bindgen
>>> +see `char` on the C side it will generate `std::ffi::c_char` which has its own
>>> +problems.
>>
>> Yeah, we definitely don't want to be using "std::ffi::c_char" in our
>> rust implementations. I do wonder if we might want to use it (or CStr)
>> judiciously in function parameters and immediately convert it to u8 in
>> the function body where the function is called from C though.
> 
> That's basically the design pattern I've been using.
> 
> In many of my translations from C to Rust I create a Rust stub
> function that takes pointer types and wraps them into safe types which
> then get handed off to a safe Rust function. I think that in the cases
> where CString/CStr is required the Rust stub function would create a
> &[u8] slice for the safe function to operate on.

That sounds like a good pattern - we get a nice interface for the C code 
and the rust implementation uses the idiomatic rust types.

Thanks

Phillip

>>> +=== Notes
>>> +^1^ This is only true if stdbool.h (or equivalent) is used. +
>>> +^2^ C does not enforce IEEE-754 compatibility, but Rust expects it. If the
>>> +platform/arch for C does not follow IEEE-754 then this equivalence does not
>>> +hold. Also, it's assumed that `float` is 32 bits and `double` is 64, but
>>> +there may be a strange platform/arch where even this isn't true. +
>>> +^3^ C also defines uintptr_t, but this should not be used in Git. +
>>> +^4^ C also defines ssize_t and intptr_t, but these should not be used in Git. +
>>
>> [u]intptr_t and ssize_t are used in git already. As Junio has pointed
>> out there are sane uses for these types but we don't want to use them in
>> structs or function parameters where the struct or function is shared
>> with rust.
> 
> You're right, I should update the phrasing. Something like: "These
> types shouldn't be used if their explicit purpose is for FFI. Whether
> as a field in a struct or part of a function signature." I'll update
> the wording.
> 
>>> +
>>> +== Problems with std::ffi::c_* types in Rust
>>> +TL;DR: They're not guaranteed to match C types for all possible C
>>> +compilers/platforms/architectures.
>>
>> Is this official policy of the rust project?
> 
> No, this is a personal inference based on logical deduction. The c_*
> definitions have changed over time with new Rust version releases, and
> Git targets more platforms/architectures than what Rust officially
> supports. While it's not guaranteed that it won't work everywhere.
> It's also not guaranteed to work everywhere either. On top of that
> we're targeting 1.63.0 who's c_* definitions are different in 1.89.0
> which I show an example of with c_long_definition. Can anyone say with
> certainty that Rust got these mappings right or wrong for all possible
> C compilers/architectures/platforms? If so (which I highly doubt)
> could someone provide a link?
>