Make @(deferred_*) procedures require a new scope

[Dreamshade-1911]

Make @(deferred_*) procedures require a new scope

In the spirit of the 1.0 announcement and the call-to-action for suggestions, I decided to write this.
Something that has really been bugging me about the language for quite a while is the fact that @(deferred_*) attributes have been put into the language to "make UI code easier to write", but it actually doesn't.

Consider the following case:

{
    begin_menu("Menu 1") // calls end_menu through @(deferred_*)
    menu_item("1")
    menu_item("2")
    menu_item("3")

    begin_menu("Menu 2")
    menu_item("A")
    menu_item("B")

    // Both calls end up here, and you render a menu inside another.
    // end_menu()
    // end_menu()
}

There are currently two ways of solving this problem, both are ugly.

Solution 1:

{
    { begin_menu("Menu 1"); // Semicolon needed for some reason.
        menu_item("1");
        // ...
    } // end_menu()
    { begin_menu("Menu 2");
        // ...
    } // end_menu()
}

Solution 2:

{
    // Have the widget return true (Which is not always possible, or desired)
    // but could also be confusing for people reading the code.
    // Should they look at the function to see if it EVER returns false?
    if begin_menu("Menu 1") {
        // ...
    } // end_menu()
    if begin_menu("Menu 1") {
        // ...
    } // end_menu()
}

Although there are currently ways to solve it, even Ryan Fleury on his blog post "UI, Part 5" recognizes that eating the complexity cost of C macros for the purposes of making this ergonomic is worth it, in this way, which is the proposed solution:

{
    // Any proc with a @(deferred_*) attribute is REQUIRED to open
    // a new scope, not doing so is an ERROR.
    // Or, provide something to mark the scope declaration as required,
    // @(deferred_*, deferred_scope_required)
    begin_menu("Menu 1") {
        menu_item("1")
        menu_item("2")
        menu_item("3")
    } // end_menu()

    begin_menu("Menu 2") {
        menu_item("A")
        menu_item("B")
    }
}

What I don't understand is: Odin prides itself on not having implicit behavior, but the current way deferred_* works IS implicit! Forcing deferred_* procedures to open a new scope in no way violates the rules of the language, in fact, it makes it follow them even more.
Users that don't use LSP's could get clean errors when they call procedures which they have no idea calls another one implicitly at scope exit, and depending on where they do it, some nasty things could happen. Error'ing out when not defining a clear scope takes the dumb case right out, and people who still want to defer things without having an explicit scope can do the usual idiom of calling defer destroy_whatever() right after an init_whatever(), which most people do anyway because they don't want implicit behavior.

With that said, I do understand that this has been brough up before, but I'm asking you guys to seriously consider it, since it makes UI code a lot nicer to write. Not only UI code, in fact, anything which implicitly manages stacks internally through guarded function calls.

[antisaling]

in some sense you are overlooking a very nice pattern e.g.:

// inside a proc or whatever
{
    flamegraph.scope("expensive operation")
    logger.scope("scraping the whole web")
}

and those two should have @(deferred_* while at the same time sharing the same scope.

while I completely understand your suggestion it seems too narrow to only consider the case for UI and I personally don't mind using an if.

on a second read I see you also suggest having another attribute that marks the scope as required. not against it but still this seems to have some weird implications. consider

scope_name: begin_menu() {
}

adds completely different semantics to the language (because now : might be used before a function?). which can only be "avoided" by having an extra scope

scope_name: {
    begin_menu() {
    }
}

or another weird case

info: Menu_Panel_Info
handle := begin_menu() {
    defer menu_info(handle, &info)
}

which makes for a very weird read because we might want handle to only live inside the scope which is presupposed when using an if i.e.

info: Menu_Panel_Info
if handle, open := begin_menu(); open {
    defer menu_info(handle, &info)
}

reads much more cleanly IMO.

[antisaling]

They actually do use it, not just to expose as API but also actually call the functions (grep for mutex_guard or GUARD). It is a very common pattern that is used in the standard library. In core:os it is used for the temp allocators (see TEMP_ALLOCATOR_GUARD)

oh my, I didn't really see it in a simple grep. stupid me. sorry.

I'll try to implement the vet and gauge the "blast radius" then.

[RoBaertschi]

They actually do use it, not just to expose as API but also actually call the functions (grep for mutex_guard or GUARD). It is a very common pattern that is used in the standard library. In core:os it is used for the temp allocators (see TEMP_ALLOCATOR_GUARD)

oh my, I didn't really see it in a simple grep. stupid me. sorry.

No problem, happens to the best of us.

I'll try to implement the vet and gauge the "blast radius" then.

That would be good to have, then we can also get accurate numbers on how much of the std would actually be affected.

[Dreamshade-1911]

I'm willing to compromise on the -vet.
To me, just being able to control the deferred scope and having a clear indicator that the function is deferring something is already good enough. Anything else is a bonus.

@RoBaertschi You should be the one writing this proposal, you write way better arguments than me 🤣

[corleypc]

I am also on the just-keep-the-shiny-toy wagon. :)

I am a bit confused, why not just do

with f() {
    with g() {
    }
}

for multiple defers and be done with it. Keeps code lines shorter, easier to reason about which defer runs when, and to be honest that thing with the return values from 2+ functions will make my head explode. :)

[RoBaertschi]

@RoBaertschi You should be the one writing this proposal, you write way better arguments than me 🤣

Thanks! I can write the proposal if you want, when I have some time I'll make a draft in here for some feedback.

I am a bit confused, why not just do

with f() {
    with g() {
    }
}

for multiple defers and be done with it. Keeps code lines shorter, easier to reason about which defer runs when, and to be honest that thing with the return values from 2+ functions will make my head explode. :)

I can understand that it is a bit hard to understand, but it was just multiple return values. But that is something syntax wise we can work on. The reason I want something like that is for procedures that start with multiple of these scopes (like performance instrumentation, some sort of temp allocator, maybe something else) and adding two full new scopes for it with full new indentation gets a bit annoying, but if anybody has a better idea, I would be open to it. Also, you have to somehow be able to retain the return value of the deferred procedure or some very common use cases for it would be impossible with with ;) or if we all agree on no -vet flag, we can also just fallback to the old ways for those.

[RoBaertschi]

@corleypc @Dreamshade-1911 @antisaling

This would be the initial draft for the proposal. I would be happy for some feedback.
There is definitely more to be added, but I just don't know right now what is missing.

@antisaling If you are/intend to implement the -vet flag, we could then add how much code that would break in the std so that we can also mention that in the proposal.

Draft: Proposal for an explicit way to mark @(deferred_*=...) procedures

Current Situation

Named procedures have the ability to be automatically deferred at call site. This uses the @(deferred_*=...) class of attributes. The effective code that gets generated at call site looks like this without the attribute:

start_doing_thing()
defer end_doing_thing()

This has the disadvantage that you still have to remember to put defer end_doing_thing() there. To make this easier on the programmer, often a wrapper with a @(deferred_*=...) is added that makes this pattern implicit:

// Wrappers

@(deferred_none=end_doing_thing)
guard_doing_thing :: proc() {
    start_doing_thing()
}

// Then use like this:
guard_doing_thing() // no explicit defer

This pattern is often used for things like locks, instrumentation, cleanup and immediate mode ui's. Based on my experience, the usage of these wrappers tend to fall into two categories. Either their usage fits naturally into the current set of scopes or they don't. When they fit well, is often for guards that are directly in the procedure scope and apply to it over the full procedure call lifetime:

some_procedure :: proc() {
    perf.scope() // instrumentation
    sync.mutex_guard() // lock
    temp := temp_allocator_guard()
}

Some might also fit quite well into already pre-existing scopes inside a procedure:

some_procedure :: proc() {
    for value in ... { // large and expensive loop
        perf.scope() // you wanna know how expensive a single iteration is
        temp := temp_allocator_guard() // you might do a lot of allocations that you only need for the iteration
    }
}

When they fit well, they don't get in your way and do their job. But what if, for example in with an immediate mode ui, you wan't to use such a deferred procedure in a way that does not fit the current scopes:

This is a version without deferred procedures to show the intend:

// This procedure creates a window that has multiple stacks inside of it
some_procedure :: proc() {
    ui.window_begin()

    ui.stack_begin(.Y) // Stack#1

    ui.stack_begin(.X) // Stack#2
    // ...
    ui.stack_end() // Ends Stack#2

    ui.stack_begin(.X) // Stack#3
    // ...
    ui.stack_end() // Ends Stack#3

    ui.stack_end() // Ends Stack#1

    ui.window_end()
}

This can get out of hand quickly, so you probably wanna use deferred procedures for that, but you come across a problem. What do you do about those nested stacks, they would start to overlap with each other in ways you don't want:

some_procedure :: proc() {
    ui.window_guard()

    ui.stack_guard(.Y) // Stack#1

    ui.stack_guard(.X) // Stack#2
    // ...
    ui.stack_guard(.X) // Stack#3
    // ...
}

This does not work, before Stack#3 was a child of Stack#1, but now it's the child of Stack#2, because this is what implicitly happens:

some_procedure :: proc() {
    ui.window_guard()

    ui.stack_guard(.Y) // Stack#1

    ui.stack_guard(.X) // Stack#2
    // ...

    ui.stack_guard(.X) // Stack#3
    // ...

    ui.stack_end() // Ends Stack#3
    ui.stack_end() // Ends Stack#2
    ui.stack_end() // Ends Stack#1
    ui.window_end()
}

There are now two ways to work around this issue:

some_procedure :: proc() {
    ui.window_guard()

    ui.stack_guard(.Y) // Stack#1

    // Solution 1
    {
        ui.stack_guard(.X) // Stack#2
        // ...
    }

    // Solution 2
    if ui.stack_guard(.X) { // Stack#3
        // ...
    }
}

In solution 1, we create a new scope that matches the lifetime of our stack.
In solution 2, we make ui.stack_guard(Axis) return a boolean that is always true so that we can use a if statement instead which looks visually more appealing.

Solution 1 is harder to read and feels wrong, solution 2 feels like a hack. Both solutions don't really solve the problem. Also, on top of that, if we forget that this even happens, we have no indication what we did wrong, because the defer of the procedure is fully implicit, we can easily forget that it even does a defer.

Also a very similar problem shows up, when you only want to guard something specific, for example lock a mutex for a few lines of code which don't have its own scope.

The Problems

The problems this proposal wants to solve are the following:

• A deferred guard procedure is something very implicit, this can be useful, but can also make the code much harder to read/understand.
• Using an if statement for a new scope explicitly for a deferred procedure that always returns true, is confusing (why does this return a bool and what does it mean) and makes the code harder to understand.
• Using a new {} block feels bad because you need a whole line with a single { that feels just wrong and it often belongs only to the actual deferred procedure in it. And it also does not express why you needed that extra block. Which is probably why if is used instead in the standard library.

The proposed solution

What we ask for is a way to mark such deferred procedures like the if statement without it being a if statement. We would propose the a new keyword with for that. It would look something like this:

with sync.guard() {
}

Semantically, this expects a deferred procedure as an expression after the with keyword which is run inside the new scope created with its block.

This would solve all the mentioned problems from before:

• It is now very explicit that the procedure defers some code and it's explicit from where to where.
• This makes it clear to somebody trying to understand, that at the end of that scope, some more unseen code gets executed.
• There is no bogus return value that does not mean anything.
• It's visually more appealing.

This would also allow people who like the deferred procs but don't like their implicitness to explicitly mark it. This would also allow for a vet flag that would require the usage of with for deferred procedures. But such a -vet flag would require adoption of this feature across the whole standard library on every deferred procedure, so this is still up to debate.

Possible -vet flag:

• -vet-require-with disallows any code using deferred procedures without a with statement.

To support the -vet flag for deferred procedures that you don't want to open a new scope for, I would also propose this syntax:

with perf.scope()

This omits the new block and the defer is inline in the current scope.

To allow for return values of deferred procedures to be used, I also propose that you can declare or assign a variable in the with:

with temp := temp_allocator_guard() {}

// or to retain the value after
value: Type
with value = deferred_value() {}

These should also be available for the inline version of the statement.

Points still up to discussion

1. The actual keyword used.
2. A -vet flag.
3. The full syntax of the new statement (does it allow multiple deferred procedures, does it allow for variable declarations for the return values of deferred procs)?
4. Should the usage of the new statement be kept optional or should it be required (large breaking change).
5. Should the inline version of when value := deferred_value() be valid, for this it might make sense to require that the variable is declared before, so that the scoping makes sense.

Breaking changes

The introduction of a new keyword would be a breaking change for some code. This could be avoided by using an already existing keyword like do. But it would not break much code. A new -vet flag would require libraries and the standard library to adopt it for full compatibility with user who use that -vet flag.

[Dreamshade-1911]

I was thinking that we could have this as well:

with proc(); // without a scope, it works in the current, just like the implicit behavior, but the -vet flag accepts it.

The reason why I say this is because as you said, having the implicit form is nice. This is just one word to make that happen and keep the ergonomics of not having to type defer whatever() right below it when proc() already tells you what needs to be deferred.
Another reason is that I scanned the standard library for code that would break, and it's currently 107 functions that would have to be pushed one level deeper in indentation because they begin with TEMP_ALLOCATOR_GUARD.

What do you think about that?

And about your proposal. Much better than mine! Really well-written and touches all the important points.

For return variable scope, I was thinking we could allow with in two places:

ret := with proc() { // ret's scope is outside the new scope
with ret := proc() { // ret's scope is inside the new scope

But I haven't thought about the implications of that yet.

[RoBaertschi]

I thought about that, but wasn't sure if that would introduce a parsing ambiguity for this:

with procedure() // <- auto inserts semicolon here
{}

But the parser already handles that and correctly associates the { with the with(this works for if statements and so on too), so this would actually work. The ; would not even be required.

About the variable stuff, I would definitely like with ret := procedure() {, the other variant does not fit with the rest of the grammar and I don't see why you would need that. Just do an assign instead:

with ret := procedure() {} // inside the new scope
ret: type
with ret = procedure() {} // ouside the new scope

This would then resemble what you can do in an if for example.

If you are happy with this, I can add it to the proposal.

[Dreamshade-1911]

Yes, makes sense.

[RoBaertschi]

Updated!