Papierkorb / bindgen

Binding and wrapper generator for C/C++ libraries
GNU General Public License v3.0
179 stars 18 forks source link

Fix non-pointer constructors for struct like types. #121

Closed f-fr closed 2 years ago

f-fr commented 2 years ago

The goal of this PR is to support "struct-like", i.e. value-like, classes. An example are classes like QPoint or QSize from Qt. These types are effectively small pass-by-value structs. The current bindgen implementation generates fully heap-allocated classes for them. However, this puts huge pressure on the memory allocator/garbage collector if used in a tight loop, e.g. using QPoint in a paint_event (and sometimes using theses classes is obligatory). For this reason the current qt5.cr implementation has a manual wrapper around QPoint (but not QRect, QSize, QLine).

Bindgen is actually able to generate wrapper code for structs but not for methods (especially constructors) within those structs. The purpose of this PR is to handle this case. The following important changes are made:

  1. The bindgen parser always assumes that a constructor expression for some class returns a pointer. This is true for the typical "new ClassName(...)" construction, but not for structs. As a consequence the generator will (falsely) add a deference operator such that the final construction code in the binding function has the form return *(QPoint(...)). This problem is not easy to solve because of the architecture of bindgen: the parser itself replaces the return type of a constructor from "none" (a constructor returns nothing) to "Class ". This is enforced in the parsing code. All subsequent steps (processors and code generators) all see the modified return value and have no (easy) way to determine that the original return value has been "none". The solution is as follows: the code generated for the construction expression is wrapped in an bg_deref template class (new in bindgen_helper.hpp) that supports the additional dereferencing operator (the above example is changed to `return (bg_deref(...))which is now valid). The reason why we addbg_derefinstead of just removing theis that it is much easier to modify the generator code to produce additional code when theQPoint(...)constructor call is generated, but the` is created at some completely different position.
  2. The generated crystal code is also modified. First, the generated wrapper class is now "struct Class" instead of "class Class". This is done by checking if the to-be-wrapped-class is configured with kind: Struct. Second, the binding code has one problem: typically all parameters of that type are passed by value with one exception: the self argument of a method. Because only these arguments are passed by pointer but all others are passed by value, the wrapper code cannot rely on the to_unsafe method anymore (it would need to have two different return types). Therefore the wrapper code is modified to pass pointerof(@unwrap) instead of self for the self argument.

Although these changes work, it would certainly be better to support pass-by-value classes directly. They currently feel more like a hack. The advantage is that the number of changed lines is rather small.

docelic commented 2 years ago

Hm, this is interesting. Let's give this approach the benefit of the doubt and try merging it. Thank you!