Can errdefer and defer be combined?

The return is supposed to indicate the end of a function. We don’t want to deviate too far from that by allowing operations of all sorts post return.

I knew that would come up, which is why I said this:

Jesus said, “the poor you will always have with you”, and so it is with bad code. A defer |retval| block would of course (as written) be a constant capture, and it would be a reasonable guard against misbehavior to forbid a pointer capture here. This is also true:

We would get some useful patterns out of it, such as the ability to define postconditions which apply to every return statement in the function, without repetition, and the ability to use those postconditions to swap a return value for an error.

I do think it’s important that this would work on a ‘catch means release’ basis. I think a case could be made that allowing an arbitrary amount of capturing defers and errdefers, rather than, say, one of either per function, would fit the “this feature is hard to use correctly and that will result in bad code too often to justify what we’d get from it” template.

But I would also that such a case would apply equally to the ?E signature for capturing defer. Overall that approach smacks of nerfing: an arbitrary limitation which confuses new users and just becomes a thing you have to memorize, and “why can’t I capture T with defer” is a question we’ll never get to stop answering.

Zig has very few weird corners like that, and that’s a big part of why I like it so much. I’d like to round down the ones it does have, and not add more. And I’d rather have no capturing defer, than one which concerns itself only with errors and doesn’t let me run postconditional assertions.

2 Likes

At risk of stating the obvious, a programming language should be capable of handling scenarios that programmers may encounter. It does happen that clean-up procedures differ depending on whether an error has happened. The OP provided a real world example. I can easily imagine another:

Suppose we have a function that runs a query on a database. The first thing that it does is acquiring a connection from a pool. Now the next statement depends on the return value. If the rows are returned as a slice of structs, a defer statement releasing the connection back to the pool would follow. If an iterator is returned, we would use an errdefer instead.

When an error occurs, we might advise the pool to not reuse the connection in the future. A server timeout could mean the connection is no longer usable, for instance. As things stand, we can easily make this distinction only in one version of our function. For no reason at all we’re forced to employ a workaround to handle the exact same scenario just because the function in happier circumstances would return a slice of structs.

I think we may have both run out of interesting things to say to one another on this question. I’ll just point out this:

Is of course possible with the signature E!T, which is (in a useful sense) a superset of ?E. I’m aware it solves problems, and which ones, and I think it’s a partial solution where we should prefer a complete one.

Until (unless? I wonder) mere users can open and debate language proposals again, it’s all fairly theoretical. If that does happen, and I hope it will, we can all hash out the details then.

Well, I’m not making a suggestion on how I think the language can be improved. I’m pointing out an actual flaw in the language. An errdefer statement might need to capture the error not because it’s somehow an error handler. It needs to capture the error because it’s a type of defer statement. It’s just illogical for defer itself to be unable to capture the error.

As for capturing the full retval, if you can come up with an usage scenario where you’d need the non-error portion of the error union in order a clean-up, I’d be all ears. I’m pretty sure you can’t. An error set, especially an inferred one, can potentially contain information on a very broad range of concerns. It might tell us that our SQL statement has a syntactical error. It might tell us that our server has spontaneously turned into a whale. When the need to capture the error is rare, the need to capture the right side of the exclamation mark is basically infinitesimally small.

fn blah() !ReturnValue {
    var handle = try magic.getOpenFile();
    defer |ret_err| {
        if (ret_err) |ret| {
            // Can still throw errors
            try handle.close();
            // Because we're returning `ret`
            // After some post-conditions 
            assert(meetsContract(ret));
            return ret;
        else |err| {
            logger.log("some error {}", .{err});
            // Can check if the error
            // allows us to close the handle,
            // or if that would just fail again
            return err;
        }
    }
    // Do stuff with the handle here,
    // It will always be closed
}

This only works with a catch-means-release rule, and to have a rule like that, you do have to catch the full E!T value.

Kind of bulky written out like that, but easily reduced to an inline function call if that’s desired. Of course half the lines are meta-commentary which a real function would not need.

Btw, errdefer capture feature is getting cut:

I doubt defer will be getting a capture. But it’s for the best.

4 Likes

" Remove errdefer |err| syntax from Zig. Uh… yeah, that’s kinda it."
Ha funny.

I indeed did not know you could capture the error. Which we will not be able to do later on anyway.

It reminds me much of Exceptions (very bad idea they are) in other languages. I never knew (like really really know) what to do with them.

I agree that errdefer should only be there for resource cleanup. Although I am aware of the logical difficulties which can arise.

The extreme simplicity of errors in Zig I love. That’s what I always needed.

This shouldn’t be a problem, it’s actually easier than stitching together identical final blocks. The various return points just unconditionally jump to the defer logic.

On the other hand, as the issue @tensorush linked to points out, anything which could be done with a capturing defer (or errdefer), can be done with an inner function. Even weird nested stuff, which really should be avoided, could be emulated with multiple levels of nesting, and inline can be used to make the result identical for most purposes.

So it’s basically syntax sugar, and not worth complicating the language over. I didn’t care for errdefer having a capture semantics, when defer didn’t; removing errdefer |err| is a satisfying resolution of that inconsistency.

If close() fails, the only reasonable reaction is to either ignore the error or panic. Doesn’t make sense to return an error when the caller cannot do anything in response.

file.close() doesn’t happen to return an error, a better example would be something like buffered_writer.flush(), which can.

Code can always catch any error which it doesn’t want to bother the caller with. Current defer can’t return, so a fallible function call in a defer block would be obliged to do this.

But really, something like this gets almost all the benefits, without adding complexity to the language. As I said, removing capturing errdefer eliminates the idiosyncrasy with less effort and complexity, which is the way to go.

2 Likes

If OutOfMemory is the only possible error, wouldn’t it be better to remove the else branch so that it becomes a compile error if any other error gets added to the inferred error set?

1 Like

Oh wait. Looks like I circled back and doubled up on something there. I converted two of the setup function (that and Graphemes I believe) to use individual catch unreachable branches for each error, but that was getting tedious, so I started doing it like this (different link, same trick).

Looks like what happened is that I went back to GeneralCategories and set up the error filter for it even though it doesn’t strictly need it. Easy enough to change, it’s a beta after all.

I think the basic pattern here is sound however. Any error other than OutOfMemory would indicate a bug in either zg or std, which I would need to fix somehow. There’s no point in user code doing the filtering for those, and by the contract of the operation, allocation failure is the only error which makes sense to expose. So if stdlib split one of those errors into two new errors, I would want the else branch to get rid of the new one automatically.

It’s a good general point, only use an else branch if the contract stipulates that any new members would go into the else branch. In this case it does. These functions decompress a segment of memory into a data structure, the only syscalls are from the allocator and it’s otherwise deterministic.

1 Like

I had missed a trywhile I was going through setting up those filters, so I hadn’t noticed that I had almost finished doing it the hard way.

Your own example demonstrates why what you’re pushing for is ill-advised. The following is a resource leak:

    defer |ret| {
        try buffered_writer.flush();
        handle.close();
        return ret;
    }

Programmers cannot make this mistake currently.

2 Likes

That looks more like your example than anything I wrote. Considering you wrote that, calling it “my example” is some sort of rhetorical move, no?

I wonder what you’re getting out of all this dead horse beating.

This is of course false:

try buffered_writer.flush();
handle.close();
return ret;

Same resource leak. The existence of defer does not compel the use of it, and it is also the case that defer statements can have bugs.

But hey, here’s “my” example:

    defer |ret| {
        defer handle.close();
        try buffered_writer.flush();
        return ret;
    }

try can’t be used within a defer (or return) so the whole discussion doesn’t make sense. And @tensorush has already shown that we will get less captures, not more.

    defer {
        const val = try getSwitched(4);
        std.debug.print("val {}", .{val});
    }
temp24.zig:26:21: error: 'try' not allowed inside defer expression
        const val = try getSwitched(4);
                    ^~~~~~~~~~~~~~~~~~
temp24.zig:25:5: note: defer expression here
    defer {
    ^~~~~
1 Like

This discussion is completely off the rail. Somehow we’ve gotten further from a solution to the OP’s problem than before. Without capture in errdefer means that if your code needs the error you’re have to catch it yourself, in every single instance. We go from this:

    try doA();
    try doB();
    try doC(try doD(), try doE(), try doF());

To this:

    var last_error_maybe: ?anyerror = null;
    doA() catch |err| {
        last_error_maybe = err;
        return err;
    };
    doB() catch |err| {
        last_error_maybe = err;
        return err;
    };
    doC(doD() catch |err| {
        last_error_maybe = err;
        return err;
    }, doE() catch |err| {
        last_error_maybe = err;
        return err;
    }, try doF() catch |err| {
        last_error_maybe = err;
        return err;
    }) catch |err| {
        last_error_maybe = err;
        return err;
    };

Why are we forcing people to write this crazy stuff? Just let defer/errdefer statement see the error. In other systems, it’s not uncommon to expose the last error globally. In C/POSIX we have errno. In Windows we have GetLastError(). In PHP, error_get_last(). The list goes on.

Wouldn’t your example be solved by putting all of those calls in a separate function and doing a single catch on that function?

If you must do it as a single function, the best I’ve come up with is below.

    const last_error_maybe: ?anyerror = err: {
        doA() catch |err| break :err err;
        doB() catch |err| break :err err;
        break :err null;
    };
    if (last_error_maybe) |err| {
        std.debug.print("err: {s}\n", .{@errorName(err)});
    }
1 Like

I agree that this is cumbersome, but it’s somewhat unrelated to OP’s specific goal: to conditionally execute based on whether an error exists, not based on what the error is.

OP’s problem could be solved without errdefer capture or any additional boilerplate for error-returning calls:


{
    // Related to DBus message iterators
    var iter: Message.Iter = undefined;
    parent.openContainer(&iter);

    // Abandon on error
    errdefer parent.abandonContainer(&iter);

    // But close only if it doesn't error
    var returned_err = false;
    defer if (!returned_err) parent.closeContainer(&iter);
    errdefer returned_err = true;

    // Do other things with iter
}
1 Like