Skip to content

Make inline function attribute interact with defer expressions at callsites #6820

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

Closed
eriksik2 opened this issue Oct 26, 2020 · 6 comments
Closed
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@eriksik2
Copy link

Right now marking a function inline is meaningless in that it doesn't change the meaning of code, it only changes how it is compiled. (If the compiler didn't already decide to inline, in which case it doesn't do anything). This makes inline hard to use effectively without resorting to some mental heuristic like "if function short or called often then inline". But this is probably similar to the heuristic the compiler uses so it still doesn't really do anything!

Inline could be made more useful while still serving its current use case, and even become a facility for things like scoped resource cleanup and performance tracing by making defer and errdefer respect inline. errdefer/defer in an inline function would run at the end of the callsite scope. This can only work if the callee is inline since defer can reference local values that still need to be in scope when they run.

Example usecase:

pub inline fn tryAcquireDeferRelease(locked: *Locked(Resource)) ?*Resource {
    const held = locked.tryAcquire() orelse return null;
    defer held.release();
    return held.value;
}
pub fn callsite() void {
    if(some_locked_resource.tryAcquireDeferRelease()) |resource| {
        resource.doStuff();
    }
}

This is a good solution to scoped resources I think since it makes you be explicit: you have to call tryAcquireDeferRelease instead of tryAcquire in the example. And it doesn't insert any hidden cleanup code, it only reorders code according to the already existing rules that defer has.

A downside is that if you forget to mark a function inline the defers would run before returning as usual and any resource you return would be quietly invalidated, but the code would still compile if the invalidation doesn't involve a this.* = undefined;. (At least I think that would catch it). Alternative could be to have a special inline defer or something, to explicitly defer something to the caller. (But mixing inline defer and normal defer in the same function would be forbidden since it would break the defer execution ordering).

@rohlem
Copy link
Contributor

rohlem commented Oct 26, 2020

[This] doesn't insert any hidden cleanup code, it only reorders code according to the already existing rules that defer has.

As defer is defined currently, it runs at the end of the block it is defined in. This would still be the inline function.
What you're proposing would be, to lift the defer and errdefer of the inline function scope into the scope of the callsite, but that would be the call expression inside the if-condition.

if(
some_locked_resource.tryAcquireDeferRelease() //we get the optional value
//defers run now that we have this optional value
) |resource| {//if the optional result wasn't null, its value is copied (or aliased) and the if-block runs

So what we would really need is to attach the defer and errdefer to the parent scope of the if-block:

{ //add a new scope so our defers run after the if branch
 const result = some_locked_resource.tryAcquireDeferRelease()
 if(result) |resource| {
  //...
 }
} //now the `defer` and `errdefer` statements run

And this translation needs to look even different for loops, so that we run defer and errdefer after every iteration.

Resource cleanup ideas have been discussed, discarded and re-opened before, see f.e. #494, #782, #5913.
Some way to semantically pass cleanup code to callsites is ultimately what would be needed, but it's tricky to do ergonomically.
Repurposing the meaning of the inline keyword seems like the wrong vehicle to me.
EDIT: Specifically adding a variant inline defer and inline errdefer makes it clear within the definition, but the code is still required to run at "invisible" points in the caller function. Hidden control flow is something Zig explicitly tries to avoid, wherever possible.

The other perspective, that the current inline function attribute is "not useful enough" seems like a non-issue to me.
It already has a specific purpose: It forces the function to be called inline, so that it shares the caller's stack frame, which is a niche use case for assembly magic.
If we don't need it for that, we can just deprecate and remove this use of the keyword.

@eriksik2
Copy link
Author

In the example I was thinking that the defer attaches itself to the scope of the inline call's result location, so in a while-loop it would be placed at the end of the loop body. And regarding repurposing the inline keyword, I'd say that inline in Zig follows a theme of "inserting code" since it is also used to unroll loops. To me making it work with defer would be a natural extension of its current semantics. An inline defer feature also isn't inherently tied to that specific syntax, other than a function using it would be required to be inlined.

The invisible control flow is admittedly an issue, maybe it could be explicit at the callsite somehow.

@rohlem
Copy link
Contributor

rohlem commented Oct 26, 2020

attaches itself to the scope of the inline call's result location

The exact return type is an optional type, that is unwrapped into the |resource| capture.
So in this case the language would have to promise that the lifetime of the payload is not longer than the lifetime of the optional holding it (honestly don't know enough about status-quo and design considerations of this).
Copying the capture and using it after the branch, or return resource; would both need to be illegal as long as defers related to the optional it comes from are in scope.

@eriksik2
Copy link
Author

Copying the capture and using it after the branch, or return resource; would both need to be illegal as long as defers related to the optional it comes from are in scope.

Well, this goes for all uses of defer right. The problem in this case is that it is hidden at the callsite.

@rohlem
Copy link
Contributor

rohlem commented Oct 26, 2020

@eriksik2 Correct, writing the defers in the corresponding location of the caller function (and introducing that anonymous scope) leads to the same result.

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Oct 26, 2020
@andrewrk andrewrk added this to the 0.7.0 milestone Oct 26, 2020
@andrewrk andrewrk changed the title Make inline function attribute semantically meaningful Make inline function attribute interact with defer expressions at callsites Oct 26, 2020
@andrewrk
Copy link
Member

Thanks for the proposal, @eriksik2. I'm going to close this proposal because my confidence level that this will not end up making it into the language is high.

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

3 participants