RAII

[renehiemstra]

I've closed #658 [(https://github.com/terralang/terra/pull/658)] and reopened it here in order to get a clean git commit history. This second attempt is implemented from scratch.

I'll summarize the pull request and the revised design:

Objective: The point of the contribution is to add some metamethods to enable RAII in order to implement smart containers and smart pointers, like std::string, std::vector and std::unique_ptr, std::shared_ptr, boost:offset_ptr in C++.

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, leaving memory allocation in the hands of the programmer.

New metamethods: The following metamethods are implemented:

1. __init(self : &A)
2. __dtor(self : &A)
3. __copy(from : &A, to : &B)

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

I will explain the metamethods in some more depth:

A managed type is one that implements the above metamethods. In the following I assume struct A is a managed type.

• __init is used to initialize managed variables:

  A.metamethods.__init(self : &A)

  The implementation checks for an __init metamethod in any defvar statement: var a : A and applies it if it exists.

• __dtor can be used to free heap memory

  A.metamethods.__dtor(self : &A)

  The implementation adds a deferred call to __dtor (right before any return statements) for any variable local to the current scope that is not returned. Hence, it is tied to the lifetime of the object. __dtor is also called before any copy-assignment.

• __copy enables specialized copy-assignment and, combined with __init, copy construction. __copy takes two arguments, which can be different, as long as one of them is a managed type, e.g.

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

  and / or

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

  __copy can be an overloaded function. In object construction, in case of b below, __copy is combined with __init to perform copy 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

I think the above covers all the use cases of smart containers and pointers in c++. I still need to make the metamethods work properly in the case of compositional API's, like mentioned in #658 (vector(vector(int)) or a vector(string)).

First, let's verify that the CI is passing. Over the next few days I'll add some more tests to test managed data-structures.

[elliottslaughter]

Thanks for submitting this. Looks like CI is still having trouble.

You removed __move in this version?

I think it might help to go through some representative code samples for different patterns and show:

1. What metamethods does the compiler check for? (I guess all of them, all the time? But this wasn't clear.)
2. Assuming those metamethods exist, what calls get inserted where?

It sounds like the compositional aspect is still unimplemented here.

[renehiemstra]

Thanks for considering the pull request.

Regarding the CI. I forgot to uncomment the SDKROOT code for macos (issue #657 ). Also, it seems like the last two months the CI is broken. In particular, the test fakeasm.t segfaults on ubuntu. Maybe, we can give it another try.

I am preparing some code examples (shall I post them here?) In short, the metamethods are checked all the time. The check for metamethods.__dtor is done once in checkblock(...) (which checks a scoped environment) and metamethods.(__init, __copy, __dtor) are checked in several parts of checkassignment(...). These checks are cheap, especially if none of the metamethods are implemented.

I will wait with the compositional aspect until we converge to well tested and supported solution.

[elliottslaughter]

A CI job from a week ago passed, except for Nix where they recently ripped out LLVM 11 (required currently for our ARM support): https://github.com/terralang/terra/actions/runs/9168683719

Having said that I see a CI job that just ran seems to have failed localenv.t, so maybe there are some non-deterministic failures: https://github.com/terralang/terra/actions/runs/9263364010/job/25481694052

I'll see about getting Nix patched up and hopefully that'll clarify whether we have additional flaky tests or not.

[renehiemstra]

Thanks for looking into that.

In my latest commits I have added the compositional aspects, by recursively generating __init, __copy, and __dtor methods. I have implemented this as a separate extension to terralib in the file lib/terralibext.t.

I have chosen to make __init, __copy, and __dtor methods rather than metamethods, since there are occasions where the programmer may want to call these methods explicitly in their code.

Rather than merging with the master branch, its probably better if we merge first with a feature branch. Then others can try it out and it remains clear this is experimental.

I am generating documentation now in a separate markdown document. So we can have a technical discussion about the implementation.

[elliottslaughter]

If you merge or rebase against master hopefully the Nix issue will be resolved. FreeBSD is still unhappy but I'll need to look at that separately. For now it's fine to ignore FreeBSD in this PR.

I agree we should give users some way to test this. Let me think about this more. Mirroring a branch into the main repo is one option; another would be putting it in behind a build or run flag or variable of some sort.

[renehiemstra]

I forgot to uncomment the macos systemcode again, which is why the the CI for macos failed. Can we try it one more time?

I've added the file /lib/raii.md to document the feature. Maybe we can use that to kickoff further discussions.

[renehiemstra]

Undid accidental deletion of a line in the macos systemcode stuff. Restored to original. Now CI should work.

[renehiemstra]

Just ran the CI on my fork. Except for Windows, everything passes. Seems like the CI is not completely deterministic. I'll investigate a bit further.

[renehiemstra]

I ran CI on the master branch about a dozen times.

• Windows-2022 failed each time due to a segfault (exit code 139) right before running the testsuite.
• Docker(ubuntu-18.04, llvm-17.0.5, luajit, static=1, slib=1, cuda=0, prebuilt) failed once with segfaults in the tests fakeasm.t and localenv.t

[elliottslaughter]

Ok. I guess I need to run the equivalent tests in master to see if I can reproduce the same bugs.

[elliottslaughter]

Alternatively (since realistically I will continue to be busy for a while), you could run master in your own fork to see if you can get it to reproduce there. Given the odds are around ~10% you'll need to run it a fair number of times (in expectation) to see the failure, if it exists on that branch.

[renehiemstra]

I did exactly that. You can see the results here

[elliottslaughter]

I put in a couple PRs to try to address the fakeasm.t failure, which I think are not actually working. (But at least fakeasm.t uses snprintf now.)

My current thought is to try to build LLVM 17 with Address Sanitizer to see if I can catch anything. I'm not sure whether it will actually work because LuaJIT tends not to play nice with such tools. My initial attempt building Terra only with Address Sanitizer tripped immediately over some LLVM initialization routine, which was not helpful.

[elliottslaughter]

I have tried a couple different variations on builds with Address Sanitizer on Linux x86, and have been unable to reproduce any crashes at all. Not sure what that indicates. My Mac builds in #662 also exhibit the same behavior: when I build LLVM with Address Sanitizer, all my crashes go away.

[elliottslaughter]

I fixed this in #662 so hopefully those crashes will go away now.

[elliottslaughter]

And the FreeBSD build should also be fixed now.

[renehiemstra]

Fantastic! I'll have time to check this next week.

[renehiemstra]

Great! CI seems to be working.