Skip to content

Comments

Add Boolean support#19

Open
rutenkolk wants to merge 17 commits intoIGJoshua:developfrom
rutenkolk:boolean_support
Open

Add Boolean support#19
rutenkolk wants to merge 17 commits intoIGJoshua:developfrom
rutenkolk:boolean_support

Conversation

@rutenkolk
Copy link
Contributor

Hi, In this pull request I propose adding boolean support to coffi via the built in boolean support from the new Java 22 FFI API.

One thing to consider with respect to boolean support is how this is implemented for different platforms and look at how the JVM defines booleans in this context. A stable C-API is desireable and booleans are notoriously an issue in this regard, so this before merging this PR these concerns have to be adressed.

For this reason I consider this PR a draft PR for the start.

@rutenkolk rutenkolk mentioned this pull request Mar 3, 2025
@fricze
Copy link

fricze commented Aug 4, 2025

Hi @rutenkolk I'd love to see this merged. Booleans would be quite useful. What help is needed to push it forward?

@rutenkolk
Copy link
Contributor Author

Hi @fricze !

The main concern with boolean support is ABI stability. The way booleans are implemented in javas FFI/M API is that they use the layout of the carrier type (a java boolean). This means java completely ignores what a boolean is for the underlying platform and even if we know the platform, different compilers sometimes use different (mutiple) byte sized values for booleans for the same platform while java just treats them as 8bit values, no matter what.

ABI stability is a pain point even without booleans, but this is probably the clearest spot where it is an issue and potentially source for bugs in client code.

I'm currently working on an experimental coffi binding generator and have my own solution / workaround for bools.
This allows for ::bool (8 bit bools) but also more specifically [::bool 8], [::bool 16] and [::bool 32], sidestepping the whole "how big is a bool?" question by answering it explicitly :
https://github.com/rutenkolk/coffimaker.runtime/blob/dfe5fb0911dbbc02f3a54c69cee4c2d64d4065a4/src/clj/coffimaker/runtime.clj#L54

Maybe such an approach would be favourable for upstream coffi as well.

I'm open to suggestions and what others think. Ultimately I think @IGJoshua has to call the shots on this one.

@fricze
Copy link

fricze commented Aug 5, 2025

@rutenkolk thanks for detailed explanation and linking your solution. I was not aware that mapping bools is that complicated.

@IGJoshua is there anything that could be done to help you here, or is it just a matter of your decision? And do you have handling booleans in any nearby future plans?

@IGJoshua
Copy link
Owner

IGJoshua commented Aug 5, 2025

Hey @fricze , I'm not opposed to having some way of doing bools in coffi, but I have concerns about just mapping to the underlying JVM "Boolean" type, because that will I think be the source of many crashes for devs starting work with coffi.

Specifically, in the C world the official "_Bool" type is new and not very widely used, nor considered very portable yet because it relies on a newer standard.

As a result, nearly every project makes its own decision for what a bool should be, and it ends up being everything from 8 bits with the only valid states being all 1s or all 0s, to 32 bits where any bit being a 1 is true, to being two different possible states among many in an enum, etc.

The varying size and the project-specific nature of these makes it pretty much impossible to make a truly generic "default" that would work for most libraries.

This will lead to users of coffi being frustrated because of JVM crashes as a result of using the default coffi bool in a call to a function which uses any other represebtation for bools.

I would however be open to a bool type which takes a bit or byte size parameter and which errors when used without that parameter. It should use all 1 bits for true, and all 0 bits for false, and when deserializing treat any 1s in bits as truthy. I think that should give it the broadest applicability for the serdes.

@rutenkolk
Copy link
Contributor Author

I'm on it. Will ping again when i'm done changing the PR to reflect this.

@rutenkolk rutenkolk marked this pull request as ready for review August 12, 2025 10:38
@rutenkolk
Copy link
Contributor Author

rutenkolk commented Aug 12, 2025

I think this PR is ready so far. I added tests to make sure everything works. I handled each case of [::bool N] separately, so as to introduce as little overhead as possible. I tried calling functions with boolean array arguments but the FFI API doesn't support any sort of SegmentLayout in function descriptors, so I just added tests to check if bool arrays serialize to the bit-patterns we would like to see.

@ertugrulcetin
Copy link

@IGJoshua seems like a nice addition, what do you think?

@IGJoshua
Copy link
Owner

IGJoshua commented Jan 9, 2026

@ertugrulcetin I agree, I will be reviewing it when I have time to work on coffi again, hopefully soon.

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.

4 participants