Can errdefer and defer be combined?

Explain
1

I’ve got an interesting case where I want to be able to use both defer and errdefer but for different paths. The idea is that if there is an error I want to call one method and if it is successful call another at the end.
As far as Zig is concerned, this isn’t an issue right? It will see two defers and call them at the right time.

Example:

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

    // Abandon on error
    errdefer parent.abandonContainer(&iter);
    // But close only if it doesn't error
    defer parent.closeContainer(&iter);
    // Do other things with iter
}

I guess in this case it depends on the implementation of closeContainer, right? If it is okay to call closeContainer after it has been abandonded, then this should be fine, I think. But if not, then I have to work around that.
Am I missing any other nuance here?

2

Language Reference: defer

Executes an expression unconditionally at scope exit.

It always gets called.

3 Likes
3

The other way around. defers are executed in reverse of the order they are defined (see defer_unwind.zig in the language reference).

5 Likes
4

defer makes it easier to write code which acquires a resource, and the code which releases, one after the other. This helps keep track of what’s going on, it’s easier to forget to release something after writing the whole method, compared to putting on the line under its acquisition.

Still, sometimes the clearest way to express “close this but only if the function completes without errors” is to make closing the last statement before returning, and just not use defer. If there’s only one return point, that’s fairly easy… but then you have to keep it that way, or remember what you did when it changes.

The documentation used to have a section which illustrates some pitfalls of errdefer and how to handle them, which shows some ways to use conditional logic which apply to defer also. For instance, you could have a guardian boolean clean_return or the like, have errdefer set it to false, and check it before closing.

But the inside-out way in which defer / errdefer executes takes some getting used to, it’s best to do the simplest thing which works, at least until that stops working.

7 Likes
5

The ideal solution would be allowing defer to capture an optional error:

defer |err| {
    if (err != null)
        parent.abandonContainer(&iter)
    else
        parent.closeContainer(&iter);
}
9 Likes
6

I’ve needed this many times.

1 Like
7

I’ve run into situations too, typically when I need to log a function’s outcome for debug purpose.

8

So as far as I know it’s possible to capture an error with errdefer by just doing the normal capture syntax errdefer |err| {...} and then you can log there.

The problem I think with attaching a capture to the normal defer woulb be, that it could potentially introduce a branch on every single one, if the compiler isn’t that smart.

2 Likes
9

It would be the same as it is with normal code

if (a) //something
if (a) //another thing

is trivial to optimise into one branch

if (a) // something
//in between thing
if (a) //another thing

is not able to combine branches unless the compiler can prove each thing doesn’t affect the next thing, which it very well could

this problem is irrelevant to defer as it persists if you write the deferred code manually

10 
var maybe_err: ?anyerror = null;
defer if (maybe_err != null) {
    parent.abandonContainer(&iter);
} else {
    parent.closeContainer(&iter);
};
errdefer |err| maybe_err = err;

Not great and open to programmer error if you’re careless, but it kinda works. Just be aware that each individual defer statement that observes the error must be immediately followed by an errdefer |err| maybe_err = err statement.

5 Likes
11

If you need to handle both error and non-error specifically, it seems easier to use if with error union:

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

if(parent.doSomething(&iter)) |success| {
    // do something with success
    parent.closeContainer(&iter);
} else |err| {
    // log error or discard it
    parent.abandonContainer(&iter);
}

I think this may result in clearer code than trying to have complicated interleaved multiple defer and errdefer that are interacting and thus more difficult to read, just to avoid indentation or having to create helper methods for code that is shared between the branches.

2 Likes
12

So, assuming that closeContainer is safe to call regardless of whether abandonContainer is called or not:

defer parent.closeContainer(&iter);
errdefer parent.abandonContainer(&iter);

If the opposite is not true (i.e. abandon cannot be called on a closed container), the this would be an logic error?

errdefer parent.abandonContainer(&iter);
defer parent.closeContainer(&iter);
1 Like
13

Yes, this would definitely be ideal in my case. You could even simplify it to

defer |err| {
  parent.abandonContainer(&iter);
} else {
  parent.closeContainer(&iter);
}

Or to mirror if with errors:

defer {
  parent.closeContainer(&iter);
} else |err| {
  parent.abandonContainer(&iter);
}

If we are bikeshedding the syntax here, I think the first is more clear and the second aligns more with current syntax for error capture.

14

fyi, a lot of these variations were discussed in Proposal: Optional else clause on errdefer · Issue #8190 · ziglang/zig · GitHub

2 Likes
15

I like what we get from this, but the semantics seem not-right to me.

I wrote an issue (closed under ‘no language proposals’ policy, which, fair) about changing the semantics of errdefer |err| to actually capture the error. Meaning that an errdefer block cannot return, but an errdefer |err| block must return.

If it worked that way, then defer could have an analogue:

defer |ret| { 
    if (ret) |ok| {
        parent.closeContainer(&iter);
        return ok;
    } else |err| { 
        parent.abandonContainer(&iter);
        return err;
    }
}

This wouldn’t come for free: if you think about the control flow implied by several nested defer and errdefer blocks like this, it could get very gnarly and hard to follow pretty quick.

But I don’t like “capturing defer gets an error but only if one was thrown”, it seems too arbitrary. The relationship between errdefer and errors is naturally mirrored by the same relationship between defer and returned values, giving defer an ?ErrorSet (which is a weird type) seems synthetic.

Given that we can do this:

That semantics amounts to syntax sugar for this one. Not quite, it would in fact be easier to use, but, close.

My main argument for changing errdefer |err| is that every other use of |cap| in Zig actually captures what it shows. In errdefer |err| it just borrows it to get a chance to look at it.

“Catch and release” defers are also a bit weird in that it would break the invariant that only one return statement executes per a call. But I don’t think it does so in a bad way actually, because return already means “run all the deferred blocks and then return” so the “re-throw” we’re talking about here isn’t all that exotic.

It might make sense to limit a block to one capturing defer, error or otherwise, or even one per function, on the premise that more than this leads to code which no mere human can understand. Maybe even require it to be the first one defined, therefore the last to run.

I’m not generally a fan of arguments which boil down to “someone can use this to write bad code”, which shouldn’t be confused with the superficially similar “this feature is hard to use correctly and that will result in bad code too often to justify what we’d get from it”.

But it’s unclear to me where any variation of this idea lies along that axis.

2 Likes
16

Here’s how I see it: Within the function scope we have a hidden variable–the return value. This variable is set just prior to the execution of some deferred clauses. The question before us is whether this variable should be visible in these clauses. Currently, only errdefer clauses can see the retval, whereas defer clauses see nothing.

But why should we privilege errdefer in this manner? errdefer clauses aren’t error handlers. The error triggering their execution is generally totally tangential to what these clauses do. Only on rare occasions would you need to know what the error is. Such occasions arise for defer clauses too because they perform the same work as errdefer, namely releasing of allocated resources. Sometimes the freeing of resources has to be done slightly differently when an error was encountered as in the OP’s case.

Allowing defer to capture simply removes this artificial disparity. We’re talking about the same variable, a 16-bit integer denoting an error. For defer clauses it can be zero, so it needs to be optional.

2 Likes
17

How I see it is that parity between errdefer and defer is best reached when, given errdefer |err| captures E, defer |retval| captures E!T, rather than capturing ?E.

This being because errdefer runs for E and defer runs for E!T, that is, unconditionally.

Of course, that’s most useful if capturing defers must return something. Which I think would be the more valuable interpretation, in addition to being most consistent with the other uses of captures. If you capture something, it’s yours now, so the block is now responsible for returning it, or something else, depending.

That would be a major change to the language, but quite possibly for the better. defer |maybe_err| having the type ?E is a more modest change, but to me it seems more expedient than rigorous.

Would the ?E version be better than the current “no defer captures” policy? Yeah, probably. But in for a penny, in for a pound.

1 Like
18

Capturing the full retval would allow programmers to do way too much. Can be useful at time for debugging purpose but the potential for misuse is just too great.

19

I mean, defer just runs some code at the end of the scope. You do have access to the full retval if you put that code at the end of the scope by hand. Giving you this option as part of defer really doesn’t change what you can do. Am I missing something?

4 Likes
20

Well, defer does more than just put code at the end of a scope. If I have multiple return sites and multiple places where the code can error, then modeling the correct code without defer would be more involved than just moving the line of code to the end of the scope or function. You could use a labeled continue in a switch statement to model the correct control-flow and fall-through behavior, but I think it’s fair to say it would be less trivial than using defer.

I don’t think anybody here is talking about expressing logic that is otherwise impossible in Zig. They’re debating whether it should be easier to express certain logic.

7 Likes