Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ability to annotate functions which allocate resources, with a way to deallocate the returned resources #782

Closed
andrewrk opened this issue Feb 22, 2018 · 78 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

This pattern is extremely common in zig code:

    const err_pipe = try makePipe();
    errdefer destroyPipe(err_pipe);

    var in_file = try os.File.openRead(allocator, source_path);
    defer in_file.close();

    var atomic_file = try AtomicFile.init(allocator, dest_path, mode);
    defer atomic_file.deinit();

    var direct_allocator = std.heap.DirectAllocator.init();
    defer direct_allocator.deinit();

    var arena = std.heap.ArenaAllocator.init(&direct_allocator.allocator);
    defer arena.deinit();

Generally:

    const resource = allocateResource();
    defer deallocateResource(resource); // or errdefer

This proposal is to

  • make it harder to forget to clean up a resource
  • make it easier to clean up resources

Strategy:

  • Functions which allocate resources are annotated with the corresponding cleanup function.
  • New keywords, corresponding to the defer keywords:
    • clean corresponding to defer theCleanupFunction(resource);
    • errclean corresponding to errdefer theCleanupFunction(resource);
  • If you want to handle resources manually, you must use noclean to indicate that you accept
    responsibility for the resource. Otherwise you get error: must specify resource cleanup strategy.

The above code example becomes:

    const err_pipe = errclean try makePipe();
    var in_file = clean try os.File.openRead(allocator, source_path);
    var atomic_file = clean try AtomicFile.init(allocator, dest_path, mode);
    var direct_allocator = clean std.heap.DirectAllocator.init();
    var arena = clean std.heap.ArenaAllocator.init(&direct_allocator.allocator);

How to annotate cleanup functions:

// std.mem.Allocator
fn create(self: &Allocator, comptime T: type) !&T
    cleanup self.destroy(_)
{
    const slice = try self.alloc(T, 1);
    return &slice[0];
}

// function pointer field of struct
allocFn: fn (self: &Allocator, byte_count: usize, alignment: u29) Error![]u8
            cleanup self.freeFn(self, _),

// std.os.File
pub fn openRead(allocator: &mem.Allocator, path: []const u8) OpenError!File
    cleanup _.close()

Having functions which allocate a resource mention their cleanup functions will make generated documentation more consistent and helpful.

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Feb 22, 2018
@andrewrk andrewrk added this to the 0.3.0 milestone Feb 22, 2018
@kyle-github
Copy link

Er... wasn't that something I proposed a few months ago in the discussion on resources and the use of "#" etc? I cannot seem to find the issue :-(

@Ilariel
Copy link

Ilariel commented Feb 22, 2018

@kyle-github I think it was #494

@kyle-github
Copy link

@Ilariel, Ah, right. Thanks! I looked back, but not that far.

I would like to see something like this proposal combined with some of the ideas in #494. I think (not carefully thought through!) that it might be possible to come close to Rust's ownership/borrow checker in power. Perhaps it is too easy to allow escapes for it to be workable, but even 90% coverage would catch a huge number of cases. Determining lifetime is not that simple, however.

@tjpalmer
Copy link

tjpalmer commented Feb 23, 2018

@andrewrk I was imagining something like this though with different keywords, but these are good keywords, too. Perhaps also allow a default standard (like your common self.deinit) so all you have to do is say clean(up) on the function header if you conform?

@kyle-github I also have some ideas about the "90% coverage" for borrow checking kind of thing, too, but I'd rather just see 1.0 first.

@hasenj
Copy link

hasenj commented Feb 27, 2018

I know this is bike shedding, but it reads kind of weird:

clean try get_something();

Sounds like "cleanly attempt to get_something()".

Maybe something more like auto_close reads more natural.

What exactly does errclean do? It sounds like "if no function call here annotated with try returns an error, then don't auto close this resource at the end of this function", which sounds like you're implicitly taking owner ship of the resource without explicitly saying so.

Maybe the default thing should be: if the function "throws" and the resource has not been assigned to any object that lives outside this scope, it's automatically closed, and there's no need to add any annotation for that.

If you want the resource to not autoclose (even on errors), that sounds like it needs a special syntax, for example: own or take.

Another idea is to mention the cleaning strategy after:

try get_something() auto_clean;

And if desired maybe it can be customized:

try get_something() auto_clean(cleanup_function)

And if no cleaning is needed:

try get_something() without_clean;

But at this point it feels like the language is getting too complicated.

@tjpalmer
Copy link

@hasenj Interesting. My keyword plan for autoclean was own, which you intuitively feel would mean the opposite. And my noclean was disown. So many different ways to take implications (and I guess that's why bikeshedding).

Personally, I still think the keywords clean, noclean, and so on from the proposal are clear. And I'm much happier with the prefix syntax, too. The object of the keyword is clearer to me. I read clean try as "autoclean" (or even "be clean with") "the thing I tried and succeeded on" and errclean as "autoclean this on error" and I'm quite happy with the "errdefer" symmetry.

@tjpalmer
Copy link

On the other hand, with this proposal in place, you could possibly drop the ad hoc defer and friends entirely.

@ghost
Copy link

ghost commented Jul 12, 2018

This proposal is to

make it harder to forget to clean up a resource
make it easier to clean up resources
Strategy:

Functions which allocate resources are annotated with the corresponding cleanup function.


that is pretty much RAII so why reinvent the wheel?

and if you basically add RAII to the language you probably need copy vs move semantics as well

@isaachier
Copy link
Contributor

So the main critique of RAII is that it is type based. That means you need to define a type for each lock/unlock, open/close, allocate/free, etc. Then again, idk a better alternative or how this is any different.

@ghost
Copy link

ghost commented Jul 12, 2018

IMO you want ctor/ dtor and with that RAII semantics sometimes and also want defer keyword other times.

Maybe just do it like rust does RAII, which is easier than cpp.

@Hejsil
Copy link
Contributor

Hejsil commented Jul 12, 2018

@monouser7dig
RAII requires constructors, destructors, move semantics overridable copy semantics and wrapping everything in wrapper types (unique_ptr).

This solution does not require any of these features, because either:

  • You clean on scope exit. Aka you own this resource and is not gonna pass it up
  • You errclean on scope exit. Aka, you own this resource when an error occurs.
  • You noclean, and is expected to pass the ownership.

@Hejsil
Copy link
Contributor

Hejsil commented Jul 12, 2018

Also, Rust RAII is easy, because Rust keeps track of ownership for you. Zig does not, so it would have to be as involved as C++.

@ghost
Copy link

ghost commented Jul 12, 2018

Well that is just a stripped down version of RAII

  • You clean on scope exit. Aka you own this resource and is not gonna pass it up (normal behavior of a ctor dtor)
  • You errclean on scope exit. Aka, you own this resource when an error occurs. (normal behavior of a ctor dtor, that is where RAII comes from)
  • You noclean, and is expected to pass the ownership. (you leave out the dtor in which case you can just not use RAII in the first place and just do it as you currently do)

so the first two cases would be covered by the traditional RAII approach and the proposed syntax is just another syntax for doing it as far as I can see.

I don't see why you would not just call it what it is.

@Hejsil
Copy link
Contributor

Hejsil commented Jul 12, 2018

@monouser7dig
Well, if this is just about the name, then sure, we can call it RAII. One should just be careful that people don't confuse it with ctor/dtor, move, copy, implicit dtor calls, wrapper types and all that.

@ghost
Copy link

ghost commented Jul 12, 2018

I argue what andrew is proposing already is ctor dtor wrapper type and soon also needs to be copy and move.
That is just how it is/ what you need.
All those functions return values and those are the wrapper types.
The „make**“ Funktion is the ctor of that type and the deferred / clean function is the dtor.

Now as soon as you copy such a type that was returned from „make**“ you need copy and move semantics as welll otherwise this example won’t hold for anything but trivial code.
....or rename it to noclean which may cover part of the usecases but it’s still reinventing the wheel as far as I can tell.

Concerning rust:
What you say is true but does not mean zig could not do the same or a variation of it. Zig does not control you memory safety either so it could just not control your moved from values and be fine, just different safety level than rust.

@bjornpagen
Copy link

bjornpagen commented Jul 13, 2018

I very much agree with @monouser7dig. As long as Zig aims itself to be an applicable alternative for C, I feel that this feature is too high level to be of good taste. It just feels like unneccesary sugaring to me. The way Zig does resource aquisition/destruction now is nice and elegant, and trying to imitate Rust and C++ here feels like a stab in the back to C-style simplicity of Zig.

make it harder to forget to clean up a resource

Is this actually a problem for anyone? This is a valid concern, but I feel that this problem should only be addressed if it is a real-life problem, not just a hypothetical.

make it easier to clean up resources

...therefore locking programmers into a single form of deallocation. There are many ways to have a "constructor", and depending on the problem, there may be many ways to have a "destructor" too. It is not the place of Zig (or any sane language) to force one form of resource destruction on the programmer.

Zig as a language tries very hard to not hide allocations behind a programmer's back. It must also not hide deallocations either.

@andrewrk
Copy link
Member Author

Too complicated

@ghost
Copy link

ghost commented Jul 14, 2018

Not sure that is the correct final answer to the problem

Language design might be complicated if it makes the programmers life less complicated in the end.

But maybe it’s best to think about it more and start a new proposal in the future.

@ghost
Copy link

ghost commented Jul 14, 2018

I think It would be especially worth to investigate #782 (comment) this issue further because #782 (comment)

@thejoshwolfe
Copy link
Contributor

Here's some real actual C code that wants to document ownership semantics for an array of strings returned by a user-supplied function: https://github.com/thejoshwolfe/consoline/blob/2c5e773442f89860f9ee82e13978b5ef3972ca99/consoline.h#L29

if this api were rewritten in zig, would it be possible to encode the desired ownership semantics with this proposal?

@isaachier
Copy link
Contributor

@thejoshwolfe presumably it would by providing a default cleanup wherever caller deallocation is necessary. I guess the assumption is that no caller should free anything provided by a function unless it has a specified clean function.

@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
@pjz
Copy link
Contributor

pjz commented May 27, 2022

Better/more @Frame introspection/definition might get us a context-alike. Consider:

fn create_window(allocator: *std.mem.Allocator) void { 
    var w: *Window  = ...;
    suspend {}
    allocator.free(w)
}

// allocate the window
var window_context = async create_window();
// get a nicer alias to the window
var w: *Window = window_context.w;
// do whatever with w
...
// dealloc the window
_ = await window_context;

This treats the @Frame like a simple struct for illustrative purposes, though it could of course be more complicated. And this is just to point out that we almost have the ability to implement an equivalent to python contexts currently. I think this rabbit hole is worth exploring as the solution-space seems to have a lot of overlap with defer/RAII.

@ghost
Copy link

ghost commented May 27, 2022

@pjz
I don't understand what is gained by putting state into an async @Frame in your example. In the end you still have to call await exactly once per suspend, and the compiler has no way to verify this in general (unless it gained some new superpowers recently?).

BTW, if #6965 is accepted, then one of the main applications would be proper context-wrapping macros, so we could implement a python-like

with_window(allocator, "name", |w| {
   // do something with w
});
// closing the window is handled by the macro

without bringing async into this.

@pjz
Copy link
Contributor

pjz commented May 31, 2022

@zzyxyzz
A few points: h

  • The state is already in the @Frame, we just need some additional introspection to get at it
  • The async mechanisms are similar to the mechanisms necessary to implement defer/RAII, so solutions in one space may be homomorphic to solutions in the other space.
  • I believe the compiler will throw an error if there's an async without an await, thus keeping the writer from forgetting to await/deinit.

I hadn't read #6965 before (thanks for that, BTW), but I find that while I applaud the goal, the means end up feeling bit too abstract. What if, instead, we could just pass around stack frames or pointers to stack frames directly?
I didn't notice that the above code would become:

// allocate the window
var window_context = async create_window();
// dealloc the window
defer { _ = await window_context; };
// get a nicer alias to the window
var w: *Window = window_context.w;
// do whatever with w
...

And while, sure, I'd love some syntactic sugar to make the async/defer await bits nicer, the zen of zig is not in favor of that.
Maybe in the interest of brevity (ala try) it could be shortened to:

var window_context = async create_window() deferred;

with deferred meaning there's an implicit defer await of the results.

@SmolPatches
Copy link

I'm new to Zig and I really like some of the things that people mentioned about simplicity within this language. While the explicit mem management that Zig offers is great, I for one would welcome this feature.

Heres my take, Rust like borrow checking is awesome but it comes with its own overhead as people here have mentioned. So making it a language standard would probably upset many, especially those using zig on constrained devices. So what if within Zig there was an abstract that could be attached to blocks where they would be attached dispatched and executed within the frames of this borrow checker? The abstraction would oversee and 'own' all the data defined within its scope.
When data is freed, the owning frame would prohibit re-usage whether it's by throwing error following attempted accesses or some other way of handling the mistake.
This way anyone could opt in to its use and it'd be compatible with traditional Zig execution. Since the runtime is only added to those blocks if a value is returned to regular execution it will lose the data protections that came with the extra runtime within the block.

@nektro
Copy link
Contributor

nektro commented Oct 25, 2022

since inline fns are inlined at compile-time, perhaps this could be solved with an inlinedefer mechanism (name pending)

pub inline fn allocAndClean(self: Allocator, comptime T: type, n: usize) Error![]T {
    defer foo(); // this would run at the end of 'allocAndClean'
    inlinedefer bar(); // this would run at the end of 'allocAndClean' caller's scope

    // eg:
    var buf = self.alloc(T, n);
    inlinedefer self.alloc.free(buf);
    return buf;
}

edit: this wouldnt solve the general case though when there's another function that calls .alloc() etc in the same way that clean try would, nvm

@jamsilva
Copy link

Here's a possibility for implementing this with just 2 new keywords.

To start with, I think that the declaration syntax described in the first comment works fine:

fn create(self: &Allocator, comptime T: type) !&T
    cleanup self.destroy(_)
{...}

When used in a defer / errdefer context, cleanup becomes the function defined as the cleanup function during the declaration so that:

defer obj.cleanup;
errdefer obj.cleanup;

are exactly the same as:

defer the_allocator.destroy(obj);
errdefer the_allocator.destroy(obj);

If the compiler has the logic to desugar this and then does the check for the defer statement on the desugared version, developers are able to choose if they want to use this or continue manually calling the respective cleanup functions (since some people seemed opposed to making the exact function implicit). More importantly existing code that does this would continue to compile correctly.

Of course, to check this, there should be an escape hatch for cases where you don't want to clean up and that's why the second keyword would be needed. Something like nocleanup perhaps (used by itself in the next statement, as opposed to noclean in the same statement as in the original proposal).

@PeyTy
Copy link

PeyTy commented Feb 2, 2023

Hi! Want to share my idea of stack pointers https://gist.github.com/PeyTy/7983dd85026cc061980a6a966bc1afc2
Somewhat related to the issue (I think there was an old issue on stack pointers detection but cannot find it).
And yes, I know, that's why not opening new issue

@ghost
Copy link

ghost commented May 2, 2023

As a new zig learner having some construct that forces you to handle resource cleanup would be really great from both a memory safety and developer experience perspective.

@tauoverpi
Copy link
Contributor

I'm wondering if attaching "uses" to variables/parameters would solve this somewhat. It would involve having something along the lines of enum { infinite, linear, dead } in addition to the is_comptime flag where infinite is that we have today with any number of copies, linear allows for just one of the value which can be moved (unlike @pin) but that would destroy the original, and dead meaning the value has no uses left thus cannot be used anymore. Function would then say how they affect the resource (comsume, preserve, produce).

For example, alloc would be fn(Allocator, type, usize) Error!linear []T where linear means exactly one use; free would be fn(Allocator, consume anytype) void where consume requires that the value is linear and that it's not been consumed before (consume 1->0). For files you'd have the same but define write as fn(preserve File, bytes: []const u8) !usize where preserve means that the resource isn't consumed by the operation. linear is viral just like @pin.

There should be no implicit cast from linear to infinite, instead a builtin @linearCast provides the ability to go back to infinite from linear when desired (e.g in the case backend code can't be proven linear but use of handles given to the user should have linear lifetimes).

defer need not be special nor does errdefer as a linear value cannot be returned via error return and linear values must be either be returned (or break) or consumed by the end of scope.

References to linear things pretty much require borrowing (e.g &T and &const T) thus there needs to be a sense of ownership where stack locals are owned by the current block { ... } until returned to transfer ownership.

TL;DR - introduce new modifiers for parameters / types along with a new type linear such that:

  • linear T means that T can only be used once
  • fn(consume T) void will make T unavailable beyond this point as the resource is destroyed
  • fn(preserve T) void ensures that T is not destroyed (think of it as a token: presented but not consumed)
  • &T gives a mutable reference to T but doesn't take ownership
  • &const T gives an immutable reference but doesn't take ownership
  • const x: linear u32 = 5; const y = x; gives ownership over to y
  • const x: linear u32 = 5; while (true) { const y = x; } is a compile-error as y takes ownership
  • const x: linear u32 = 5; while (true) { foo(x); } is a compile-error unless foo uses preserve as foo consumes
    the value
  • const x: u32 = as(linear u32, 5); is a compile error as no implicit casts from linear to infinite
  • linear T is an explicit modifier, default is infinite (no annotation)

@PeyTy
Copy link

PeyTy commented May 4, 2023

@tauoverpi what the key differences from Rust's borrow checker?
To be honest, I don't see any notable differences, except the defaulting to non-linear.
And preserve feels like static lifetime from Rust too.

What you're proposing here would essentially make Zig a Rust 2.0 (😉)

@matu3ba
Copy link
Contributor

matu3ba commented May 4, 2023

References to linear things pretty much require borrowing (e.g &T and &const T) thus there needs to be a sense of ownership where stack locals are owned by the current block { ... } until returned to transfer ownership.

Declaring ownership is not the main problem. Enforcing via solving the semantics however is, because otherwise annotations only describe intended semantics.

The main problem of the Rust borrow checker is summarized here #2301 (comment), so it is a committed local optimum.

More general formal systems would boil down to more expressive annotations taking into account how the formal system is solved, which are not possible to bake into the syntax as it would mean to enclose all possible formal systems to prove all possible properties.

Something like #14656 would be necessary to cover these use cases.

@InKryption
Copy link
Contributor

InKryption commented May 4, 2023

Bringing up an idea inspired by a discord conversation from a while ago:

fn make(ally: Allocator, n: usize) Allocator.Error!unmake#[]f16 {
    const result = try ally.alloc(f16, n);
    @memset(result, 0.5);
    return result; // coerces implicitly to the obligation
}
fn unmake(
    ret: []const f16,
    allocator: std.mem.Allocator,
) void {
    allocator.free(ret);
}

test {
    const ally = std.testing.allocator;
    // `defer(...)` is required to unwrap the result, and simultaneously does the
    // same thing as `defer unmake(slice1, ally);` all the required state except
    // for the result value itself must be passed as arguments - no closures or
    // contexts or whatever else
    // `errdefer(...)` may also be used for the behavior you'd expect
    const slice1 = try make(ally) defer(ally);

    // escape hatch to request to manage the resource manually, eliding automatic cleanup.
    // syntax doesn't matter, could be anything like `foo() noclean`.
    const slice2 = try make(ally) @noclean();
    unmake(slice2, ally);

    // this should be a compile error. "clean up unions" or whatever you want to call them
    // should never be stored. They should be treated like half opaque types,
    // in that using one as the type of a field or variable should be a compile error,
    // but should be allowed as the type of an expression (e.g. result of a function call,
    // in-place coercion with `@as`, etc). This helps to avoid double frees.
    const slice3 = try make(ally);
}

The pieces that comprise this are all very similar to other concepts brought up in this issue, but I feel this addresses a good bunch of the counter-points of those previous ideas.

And to just quickly address the "no hidden control flow" argument that may be brought up against this, I would argue that there isn't any control flow that's truly hidden here, as the caller is explicitly forced to either opt-in or opt-out of the cleanup behavior, which effectively makes it as transparent as just deciding whether or not to call a function - it just happens that said function is being "recommended" to clean it up.

Edit: it occurs to me that, with the syntax presented here, pairings like ArrayList's init and deinit will work fine with the first argument rule, but wouldn't work well with Allocator's alloc and free, since the first argument to free is the Allocator. Not 100% sure how to rectify this. One idea might be to mark in the resource-freeing function which parameter receives the resource, something like

fn alloc(self: Allocator, comptime T: type, n: usize) Error!free#[]T {
    // -- snip --
}
fn free(self: Allocator, #ptr: anytype) void {
    // -- snip --
}

and when it comes to how the arguments end up passed, it should behave as if the resource argument is put between the relevant arguments to defer(...), e.g.

fn make(ally: Allocator, n: usize, align_log2: Allocator.Log2Align) !unmake#[]align(1) i16 {
    // -- snip --
}
fn unmake(ally: Allocator, #slice: []const i16, align_log2: Allocator.Log2Align) void {
    // -- snip --
}

test {
    const ally = std.testing.allocator;
    const slice1 = try make(ally, 15, 0) defer(ally, 0); // the value is passed as if in between `ally` and `0`.
    const slice2 = try make(ally, 31, 4) errdefer(ally, 4); // ditto
    const slice3 = try make(ally, 63, 2) @noclean();
    defer unmake(ally, slice3, 2); // can still call as normal function
}

Edit 2: another idea would be to make the syntax look like this:

const slice = try make(ally, 127, 0) defer(ally, _, 0);

Which may look a bit odd, but keeps things simple, and is tangentially related to inference, with underscore being the same symbol that's used to infer array literal length.

@InKryption
Copy link
Contributor

OK, a slightly revised version of the proposed ideas after some amount of deliberation:

const std = @import("std");

// #unmake is an annotation of the whole function, not part of a type (syntax up for debate,
// maybe use `@cleanup(unmake)`?)
fn make(ally: Allocator, size: usize, align_log2: Allocator.Log2Align) E![]T #unmake {
    // -- snip --
}
fn unmake(ally: Allocator, slice: []T, align_log2: Allocator.Log2Align) void {
    // -- snip --
}

test {
    const ally = std.testing.allocator;

    // compiles fine, is the same as writing
    // ```
    // const slice1 = try make(ally, 31, 0);
    // defer unmake(ally, slice1, 0);
    // ```
    const slice1 = try make(ally, 31, 0) defer(ally, _, 0);

    // compiles fine, is the same as writing
    // ```
    // const slice2 = try make(ally, 127, 8);
    // errdefer unmake(ally, slice2, 8);
    // ```
    // caller takes responsibility for freeing the resource in
    // the absence of error
    const slice2 = try make(ally, 127, 8) errdefer(ally, _, 8);

    // this is the fine, it is the same as choosing to omit the
    // `defer unmake(ally, slice4, 4);` code - caller takes responsibility
    // for freeing the resource
    //
    // Syntax up for debate, maybe use a builtin like `@noclean()`, or maybe
    // an operator using the # symbol, or whatever else. Could also be prefix
    // instead of postfix to distinguish from normal defer auto-cleaning.
    const slice3 = try make(ally, 63, 4) noclean;
    defer unmake(ally, slice3, 4);

    // // this would be a compile error, it's the same as writing
    // // ```
    // // const slice4 = make(ally, 31, 0);
    // // defer unmake(ally, slice4, 0);
    // // ```
    // // basically, `E![]T` can't coerce to `[]T`
    // const slice4 = make(ally, 15, 2) defer(ally, _, 2);

    // this would be a compile error, because the expression resulting from
    // a function annotated with a clean up function is not annotated with
    // either a `defer`, `errdefer` or `noclean`.
    const slice5 = try make(ally, 7, 1);

    unmake(ally, slice2, 8); // don't forget to clean up after no errors occurred
}

To put some of this into more concrete terms:

An annotation on a function of the form #name indicates that the result location of the expression of a call to the function is a resource that must be cleaned up. This does not type check against anything yet.

When this function is called, the caller may transform the resulting expression in whatever arbitrary way is desired; we can say that the expression itself is flagged by the compiler as being the result of a call to a function that's annotated with a clean up procedure.
If the expression is assigned to a variable, parameter, etc, without getting rid of this "flag" or "metadata" or however you'd like to call it, the compiler issues an error; the only way to get rid of this flagging is by annotating the expression with either defer(...), errdefer(...), or noclean.

  • With noclean, the resulting behavior is that of status-quo: no automatic clean up, the caller may do with the resource whatever they like (return it, clean it up manually or via a different method, etc).
  • With defer(...) and errdefer(...), the result location is represented implicitly by _, and all other relevant arguments must be passed as well. This effectively is the same as defering or errdefering the clean up function from the previously mentioned function annotation, with the aforementioned arguments. They will be as-if declared in the same scope as the result location.
    • The result location is passed implicitly by pointer or by value, with one level of dereference supported, as with method calls.
    • In the case of the result location being the discard identifier, the defered clean up function should run immediately.

In essence, this is purely a formalization of the extremely common-place pattern of paired init() and deinit(), turning code like

var list = try std.ArrayListUnmanaged(u8).initCapacity(allocator, 32);
defer list.deinit(allocator);
// -- snip --
return try list.toOwnedSlice();

into

var list = try std.ArrayListUnmanaged(u8).initCapacity(allocator, 32)
    defer(_, allocator);
// -- snip --
return try list.toOwnedSlice() noclean;

@PeyTy
Copy link

PeyTy commented May 6, 2023

@InKryption consider this, what comes at ????

fn make_at_upper_scope(ally: Allocator, size: usize, align_log2: Allocator.Log2Align) E![]T #??? {
    return try make(ally, 31, 0) ???;
}
fn make(ally: Allocator, size: usize, align_log2: Allocator.Log2Align) E![]T #unmake {
    // -- snip --
}
fn unmake(ally: Allocator, slice: []T, align_log2: Allocator.Log2Align) void {
    // -- snip --
}

test {
    const ally = std.testing.allocator;
    const slice1 = try make_at_upper_scope(ally, 31, 0) defer(ally, _, 0) /* or what? */;
}

@ghost
Copy link

ghost commented May 6, 2023

@PeyTy
You can always try to take manual control in such cases:

fn make_at_upper_scope(ally: Allocator, size: usize, align_log2: Allocator.Log2Align) E![]T #unmake {
    return try make(ally, 31, 0) noclean;
}

... which is of course unsatisfactory, because you have to dig out the literal function doing the cleanup, which might just as well be private to another module. It would be nicer if we could somehow refer to the destructor anonymously, but I don't see how this would be possible syntactically, since the destructor is attached to a call within the function body, but has to be referred to outside of it, in the signature. So this probably calls for a separate feature to forward an obligation:

fn make_at_upper_scope(ally: Allocator, size: usize, align_log2: Allocator.Log2Align) E![]T # {
    return try make(ally, 31, 0) moveclean;
}

Just thinking out loud. Maybe there's a simpler solution.

@InKryption
Copy link
Contributor

InKryption commented May 6, 2023

Obligations definitely should not be implicit, as that essentially would start to fall under hidden control flow. Perhaps it could be part of type info, and be retrieved by a helper function as #obligation(make).

@PeyTy
Copy link

PeyTy commented May 6, 2023

Speaking of hidden control flow. Most ideas around Zig memory management pretend that memory lifetime is simply linear and stack-like. I don't believe defer and automatic substitution of defer(ally...) solves everything, but I agree that Zig follows explicit rules.

Thus, what about, instead of trying to automate defer cleanup() calls, compiler would require manual action?

A quick made up example:

fn make(allocator ..., ...) #reminder {
  ...
}

test {
  var gpa = ...;
  var demo = make(gpa, ...);

  // at the end of scope compiler would say something like "demo leaves the scope, action required"
  gpa.free(@actionTaken(demo)); // @actionTaken simply removes the flag #reminder from this variable

  {
    // would work for ref counting too
    // Doc comment saying that "you must call rc_decrement at scope leaving"
    fn alloc_rc(allocator, ...) T #reminder_at_every_scope_leaving {
       return ...
    }

    fn rc_decrement(allocator, object #action_taken, ...) {
      if (object.rc-- == 0) allocator.free(object);
    }

    var refcounted = alloc_rc(gpa, ...);
   
    // you must do something according to the documentation of the `alloc_rc` function
    // in Zig it is assumed that you follow the docs anyway
    rc_decrement(refcounted); // #action_taken implicitly means @actionTaken(refcounted)
  }
}

You may read more about similar idea in Vale: https://verdagon.dev/blog/higher-raii-7drl

That part:

With Higher RAII, we can ensure that we eventually call any function...

This way you don't have any automatic substitution, but compiler helps you to not forget to do something.
It makes the code literally the same as it is right now in any Zig codebase, with all those GPA calls etc, but with compiler #reminders. Seems like a nice compromise?

@whatisaphone
Copy link
Contributor

It might be too limiting to tie the return value directly to one particular cleanup function, because some resources can be cleaned up in more than one way. For example:

  • a DB transaction can be either committed or rolled back
  • std.Thread.join and std.Thread.detach

@InKryption
Copy link
Contributor

Well, in the thread case, those are cases wherein it wouldn't make sense to call either of those functions to "release" the resource. If we look at something like C++ or Rust, which have destructors for "resources", they also do not join nor detach in the destructors of their standard thread types.
These annotations would be focused on the more common use case of resources which are tied to mostly linear scopes.

@whatisaphone
Copy link
Contributor

(It's a real minor point, but Rust does detach when you drop a JoinHandle)

If more complex resources are out of scope, fair enough, just thought it was worth mentioning

@andrewrk andrewrk modified the milestones: 0.12.0, 0.11.0 May 29, 2023
@andrewrk
Copy link
Member Author

I think having this issue open is confusing/misleading people. I'm not seriously considering this proposal and it's very likely that nothing will come of it. The only reason it was open was due to being possibly related to async cancellation. That is still an unsolved use case, but even so I'm confident the solution is not annotating resource-allocating functions with what is essentially destructors.

@andrewrk andrewrk closed this as not planned Won't fix, can't repro, duplicate, stale May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests