terralang / terra

Terra is a low-level system programming language that is embedded in and meta-programmed by the Lua programming language.
terralang.org
Other
2.71k stars 197 forks source link

RAII - memory management #658

Closed renehiemstra closed 3 months ago

renehiemstra commented 3 months ago

This pull request adds memory management to terra using the concept of RAII.

The design adds the following four metamethods to the language that enable the implementation of smart (shared) pointer types:

  1. __init(self : &A)
  2. __move(self : &A)
  3. __copy(self : &A)
  4. __dtor(self : &A)

If implemented, these methods are inserted judiciously during the type checking phase, implemented in terralub.lua. All these metamethods can be implemented as macro's or as terra functions.

The design does not introduce any breaking changes. No new keywords are introduced. Heap resources are acquired and released using the regular C stdlib functions such malloc and free.

I will explain the metamethods in some more depth:

I have added a test file smartptr.t in the tests folder. It contains an implementation of a smart shared pointer and two integration tests that test all four metamethods. These tests print out the control flow of the program to check when each meta method is invoked.

All existing tests in the tests folder pass on my machine (except for a few that did not pass prior to the changes)

If the design is accepted, there are a few things left to do:

  1. Additional testing in real use cases.
  2. Implement output for prettystring in terralib.lua.
  3. Adding a smart pointer implementation to std.t.
  4. Documentation of the feature on github and the terra website.

I am happy to work further on improving the design where needed.

chriku commented 3 months ago

I generally like the idea, but (at least) your __move returns an object, which is then copied by value by terra. If I read that correctly, this means stuff like offset_ptrs (container that saves the offset between this-ptr and the value, VERY useful for IPC stuff) are not possible, so maybe your move and copy could take a second (first?) parameter in which to move/copy the object into? Another example would be memory reuse in containers, as fully destroying the object and constructing a new one is far more costly than just reassigning the content, so an optimized move/copy assignment would be appreciated.

Also: I first thought, that you do not have a method for move assignment (of existing variables), but it's of course possible to make an assign method, so maybe elaborate in that direction to avoid confusion when skimming over the PR?

Also Also: Your RAII code does not do any memory allocation, only the not-shared_ptr test script. Maybe clarify on that topic, to avoid stirring up any memory management debates

Also Also Also: you commented out some sdkroot platform code, probably so it works on your machine, but this shouldn't be part of the PR

renehiemstra commented 3 months ago

__move

__copy

__init It may be worthwhile to have a default __init that sets pointers to nil whenever a __dtor is implemented.

Also Also: The point of the contribution is to add some metamethods to enable RAII, without introducing breaking changes. This leaves memory allocation using the stdlib C library in the hands of the programmer.

Also Also Also: right, the commented out platform code is a separate issue that I forgot to undo.

elliottslaughter commented 3 months ago

As I said on Zulip, I appreciate the work that @renehiemstra has put into this and agree that something along these lines would be useful.

Unfortunately I am continuing to run out of time to actually look at this in depth.

I think it would be nice to make sure that this can capture basic patterns similar to std::vector, std::string, etc. I suppose at a superficial level those are similar to smart pointers, but one would generally expect (a) owning semantics, not shared semantics, and (b) copies are especially painful if we don't optimize those.

I have enabled the CI to run so that you can at least see those results. I may have to re-enable it on each push since this is a first-time contribution.

Edit: I also wanted to check that the metaprogramming APIs are compositional: i.e., you can create a vector(vector(int)) or a vector(string) or etc. and have it do the right thing. I'm not saying the current version isn't, but I just haven't had time to think through it.

renehiemstra commented 3 months ago

I was able to implement a __dtor metamethod (without using defer) that is invoked at the end of each new scope, before a possible return-statement, and only acts on the variables that are not returned. As such, destruction is really tied to the lifetime of an object, which is precisely what we need. The __move metamethod and the additional copy are no longer needed.

I also implemented a __copy metamethod that takes two arguments

    A.metamethods.__copy(from : &A, to : &B)

and / or

    A.metamethods.__copy(from : &B, to : &A)

The arguments of __copy can be different and it can be an overloaded function.

It can also be used on object construction,

    var a : A
    a.data = ...
    var b = a

which is the same as writing

    var a : A
    a.data = ...
    var b : A
    b = a

The __init metamethod is called in object initialization, if it is implemented.

I think the above covers all the use cases of smart containers and pointers in c++ (e.g. std::vector, std::unique_ptr, and std::shared_ptr) and it should also work for the case of boost::offset_ptr

I still need to make the metamethods work properly in the case of compositional API's, like you mentioned (vector(vector(int)) or a vector(string)).

Also, I need to handle some corner cases. In Terra its allowed to combine a logical statement in a return statment, e.g.

    return some_bool_flag and x or y

I am not sure how to deal with these yet, because this is runtime information.

renehiemstra commented 2 months ago

Reopened in #659