freebasic / fbfrog

FreeBASIC binding creation tool
Other
35 stars 17 forks source link

Convert _Bool to boolean #6

Open rversteegen opened 3 years ago

rversteegen commented 3 years ago

To my surprise, on examing the assembly/C, I realise that a boolean is stored as a value of either 0 or 1 in a byte, and is converted to/from 0/-1 when converting to/from a numerical type. This appears to mean it is exactly the same as C's _Bool (or bool with stdbool.h).

Therefore, why doesn't fbfrog convert _Bool to boolean? The fact that it instead translates to 'byte' breaks code such as while not WindowShouldClose() (function returning a bool) from the raylib examples and all previous translations of those examples and headers into FB, so I feel I have no choice but to edit my new translation of raylib.bi to use boolean.

Boolean was only added in 2015, so I guess it was just never implemented. I'm currently working on this, and have come across no problems so far. The only concern is whether there's any case where this would break code using existing headers, if the headers are regenerated.

rversteegen commented 3 years ago

Well, I suppose there is one major problem: converting a _Bool declared in the .h/.bi to an integer or other numeric type in FB. Using byte results in 0/1 while using boolean results in 0/-1. (When this conversion happens in code that fbfrog itself translates, it can convert to 0/1; the issue is in non-translated code.) Perhaps it should be a commandline-togglable option so some existing headers can remain as 'byte' for compatibility if the risk where the change is deemed too great. I'll check now how many headers actually change.

rversteegen commented 3 years ago

Here are the results of regenerating fbbindings with my branch: Many changes: allegro5, ncurses, llvm-c (lto_bool_t only, which is used a lot), raylib Few changes: gif_lib5 Nonbreaking changes only: gif_lib4 (function args), libvlc (function args), Newton (return type of a callback passed to the library - I assume OK)

There were no changes due to the addition of (_Bool) cast support or _Bool->long casting in expressions to fbfrog; all changes were in the types of args, return values, struct members or extern variables.

This is the wrong repo to discuss this, but I suggest that if compatibility is a concern allegro5 and ncurses and maybe gif_lib5 bindings continue to use byte instead of boolean, and all the other headers switch to boolean. llvm-c bindings are way out of date anyway, and raylib bindings are not yet in fbc so can be changed. Ideally there would be opt in or deprecation process for these headers.

dkl commented 3 years ago

Agreed, since FB has a C-compatible boolean type now, it can be used in translations too.

fbfrog even already has some support for turning (a == b) + 1 into (-(a = b)) + 1 to convert 0|-1 back to 0|1 in mathematical expressions, which can be extended to cover some cases involving _Bool aswell. But it probably can't and shouldn't wrap every function returning _Bool with a #define f_fixed(x) (-f(x)), so while FB's "logical" operators (not, and, or...) will then work, the "math" operators will no longer work. I think that's acceptable though...

rversteegen commented 3 years ago

I think that changing a header to use boolean instead of byte is very unlikely to break any individual program using it, and more likely to fix rather than break untested code, but there's probably some code that will break. I don't know how cautious to be. Using non-logical operators on booleans returned from functions will trigger an error, so the worry is just implicit integer conversion on assignment. I definitely wouldn't want to create function wrappers, might as well just disable _Bool->boolean translation entirely.

Changes to the default behaviour of fbfrog can be made pretty freely, though.

I've force-pushed my branch with tests and better handling of _Bools in expressions. It's now part of the 0|1 -> 0|-1 handling. However because ExpressionFixUp.calculateCTypes doesn't actually deduce the type of a function call expression nor of a variable, it seems currently the only _Bools that the code considers are due to (_Bool) casts, of which there seem to be zero in fbbindings.