std: replace std::unstable::finally with a RAII finally! macro

[huonw]

This adds a scope! macro similar to D's scope statement. It runs a
user specified piece of code in the destructor of a stack variable to
allow guaranteeing that it runs, even on failure.

[huonw]

I feel like the name of this could be improved; I just borrowed it from D without much thought.

Maybe something like on_exit!(...) or at_scope_end!(...). Does anyone have any other suggestions?

[ghost]

Go uses the defer keyword.

[zr40]

-1 for on_exit -- one might think the code would be called when the process is about to exit.

My suggestions: leaving_scope or exiting_scope.

[ghost]

Is this release-before-acquire pattern intentional? Would it make sense to have a two-argument macro with a "constructor" expression?

[thestinger]

@Jurily: The defer keyword in Go is based on functions rather than block scoping, so it's not really the same thing.

[omasanori]

I'd suggest scope_exit! (for similarity with D's scope(exit)) in addition to the candidates that have been suggested already.

[alexcrichton]

cc #9842, which sounds like there may be soundness issues around this strategy. That being said, this is the same strategy as finally, so I don't think we should block on that.

When we initially discussed scope awhile back (and with #9842), it gave us the ability to run functions on exit, failing, or success, and that seems like useful functionality. I suppose the macro could include the literal exit fail or success tokens. It does seem a little odd when reading for the first time:

foo.lock();
scope!(foo.unlock());

It doesn't quite look like the foo.unlock() will happen at the end of the scope.

scope!(exit, foo.unlock());

That also looks a little silly because there's no indication of what exit is, sadly. All in all, this is probably worth munging for a bit to see if we can get happy with the names/syntax, but I think we should definitely merge this.

[lilyball]

Maybe just call it finally!()? Or alternatively, using defer!() is not terrible, even though it isn't quite the same as Go's defer keyword.

[lilyball]

Reading #6801 (referenced from #9842), it sounds like fixing that issue will make this approach significantly less useful, because any upvalues referenced in the closure will become active borrows for the lifetime of the closure. This may be the same strategy as finally, but that was in std::unstable whereas this PR makes the functionality public.

[brson]

I personally prefer finally to defer. I don't understand why one would want to write their code backwards.

[thestinger]

The nice thing about the D/Go style is not requiring any additional nesting.

[brson]

In some of the replacements here scope! comes first, and in some it is written last. Where it's written last it looks nice, but where it's written first it really looks confusing:

// Yay!
lock.lock();
do_something_under_lock();
scope!(lock.unlock())

// Boo!
scope!(lock.unlock())
lock.lock();
do_something_under_lock();

Are there cases where borrowing forces it to come first?

[brson]

The second isn't really even correct:

scope!(lock.unlock());
// what if we fail here? the lock isn't locked
lock.lock();

[thestinger]

@brson: The first could also fail in do_something_under_lock. It needs to be directly before or directly after the lock call. It would definitely be a lot nicer to use custom destructors in any cases where it's a reused pattern.

[huonw]

The second example could be (using finally):

lock.lock();
finally!(lock.unlock())

do_something_under_lock();

which isn't entirely horrible (it's certainly better than putting it before the .lock).

(And I completely agree with using RAII/custom destructors where it happens a lot; certainly mutex lock/unlock code would be cleaner & safer with it.)

[flaper87]

FWIW, I prefer finally as well!

[huonw]

Also, I decided to avoid scope!(exit, ..);, scope!(failing, ...) etc. to mirror D, because I thought that just using if task::failing() {... } (and its negation) was short enough and more flexible.

[adrientetar]

Not speaking in terms of functionality but I like finally as well.

[huonw]

Updated with it renamed to finally!, and changed the instance of the silly backward finally!(self.release()); self.acquire() ordering to have the finally! second.

[lilyball]

@nikomatsakis says he has a patch for #6801. I'm expecting it to impact finally!(). It would be a good idea to see how finally!() behaves with his patch before committing it.

[huonw]

Sounds good (also, this has been added to the meeting agenda).

[emberian]

The macro I wrote to do this was called defer: https://gist.github.com/cmr/8401403

[brson]

To be clear, I prefer finally blocks to the defer pattern. I don't know whether I prefer finally! as the name of this macro.

[emberian]

I don't know what the defer pattern is, it just seemed like the obvious
name to me.

On Sat, Feb 1, 2014 at 6:39 PM, Brian Anderson notifications@git.luolix.topwrote:

To be clear, I prefer finally blocks to the defer pattern. I don't know
whether I prefer finally! as the name of this macro.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/11905#issuecomment-33886973
.

[huonw]

One disadvantage of the (current) implementation of std::unstable::finally is it requires putting the "normal" code in a closure as well as the always-run part (so, e.g., return doesn't work), I guess something else that could work, and mirrors in the current std::unstable::finally more closely, is

finally!(/* run this */, /* then always run this*/)

(where the first part is expanded inline, without requiring a closure.)

[zkamsler]

I think there is a definite risk of confusion in calling it finally, as people may expect it to behave the same as in other languages and put it after the thing that might fail or return (as in brsons's "Yay!" example).

[brson]

The examples here do look better as written, and most are easy to understand. It seems like where with try/finally you would write:

do_thing_that_needs_to_be_reversed();
try {
   do_other_stuff();
} finally {
    reverse_thing();
}

Now you would write

do_thing_that_needs_to_be_reversed();
{
    finally!(reverse_thing());
    do_other_stuff();
}

This is more concise, and I can see how in these common cases it is also possibly even more clear than try/finally on a top-down reading.

[nikomatsakis]

The changes for #2202 (actually #6801) to be more precise require that if a variable is mutable, it is effectively captured by the closure. This impacts try-finally patterns that update a variable in the try block and then access it in the finally block.

For example, code like this would not compile:

let mut i = 0;
(|| {
   ... i += 1; ...
}).finally(|| {
    use(i);
});

I wound up adding a new try_finally function that worked like this:

/// Invokes `try_fn(v)` and then `finally_fn(v)`. Always invokes
/// `finally_fn(v)`, even if `try_fn(v)` fails.
fn try_finally<A>(v: &mut A, try_fn: |&mut A|, finally_fn: |&mut A|) {...}

Then you can translate the above code into:

let mut i = 0;
try_finally(
    &mut i, 
    |i| { ... *i += 1; ... },
    |i| { ... *i ... });

You can extend this to multiple variables using a tuple or some such but I never had to. An alternative is to use a Cell to share data. I'm not sure how this interacts with the proposed macro.

I hope to write up the details in a blog post but I thought I'd put up a summary here briefly. There is a bit of wiggle room here -- in an earlier version, I permitted (in some cases) variables to be mutated in one closure but read in another, so long as they were not mutated in both, this was effectively bringing back const (read-only but not immutable) borrows. But I eventually removed it because it wasn't used that frequently. This is something I had hoped to write up and discuss, there are some tradeoffs involved.

[ghost]

Since we're going for RAII semantics here, why not call it raii! or even drop!?

[alexcrichton]

We decided in today's meeting to move towards this macro, but call it defer! instead.

@huonw, are you ok implementing that change?

[ben0x539]

Are you strongly opposed to a built-in try-finally construct? The complexity of having to move or borrow values into closures might justify special-casing this arguably common pattern.

[thestinger]

I think built-in scope guards would make sense, but not another block-based control structure. Rust shouldn't take a step backwards from D and Go by requiring a bunch of nesting for this. Stack-based once functions are going to feel like a missing feature if closures are used for RAII patterns like this. They were left out with the rationale that we weren't going to do this anymore...

[ben0x539]

finally blocks have the advantage of being familiar and also of coming after the code they get executed after. :/

[thestinger]

C++ doesn't have finally, but scope guards do exist as an idiom there. A finally block may be more familiar to Java or Python programmers but it's less familiar to a D, Go or C++ programmer. I don't think it's a valid argument, especially since Rust wants to appeal to C++ programmers.

Rust already has destructors, so the RAII idiom is already part of the language. Stack-based destructors are an idiom every Rust programmer is familiar with.

[ben0x539]

I'd expect C++ programmers to be familiar with finally from casual exposure to either of Java, C#, Javascript, Python, PHP, D, a dozen others I'm not aware of, moral equivalents in Ruby and maybe Haskell, etc, and to have written C++ code to emulate finally one way or the other.

On the other hand, non-C++ programmers potentially interested in Rust are a lot less likely to be familiar/comfortable with the hoops C++ programmers jump through to emulate finally.

Of course maybe I'm completely off base with that, or maybe C++ programmers make up such an overwhelming majority of the Rust demographic that it doesn't matter anyway, but I think it's worth considering just as one aspect among many contributing to this decision.

[thestinger]

On the other hand, non-C++ programmers potentially interested in Rust are a lot less likely to be familiar/comfortable with the hoops C++ programmers jump through to emulate finally.

That's a mischaracterization of scope guards. They're not an emulation of finally but rather a way of running code at the end of any block without introducing extra nesting.

Java, C#, Javascript, Python, PHP, D

D has scope guards and they're almost always regarded as a far better option than try by the D community. Avoiding nesting and keeping the cleanup code near the initialization is a good thing. Python has with statements which are somewhat similar too, but don't work for ad-hoc cases like scope guards so they're more like explicit destructors.

Of course maybe I'm completely off base with that, or maybe C++ programmers make up such an overwhelming majority of the Rust demographic that it doesn't matter anyway, but I think it's worth considering just as one aspect among many contributing to this decision.

Scope guards avoid nesting, are more familiar to C++ programmers and would only require one keyword if they were included in the language rather than as a macro. What is the advantage of finally, beyond your familiarity with it?

[ben0x539]

@thestinger I don't have anything to add beyond comments already made in this thread.

[huonw]

We decided in today's meeting to move towards this macro, but call it defer! instead.

@huonw, are you ok implementing that change?

Yes, although I guess #12158 will make this less useful unless the macro changes to mirror the try_finally function defined there.

[huonw]

although I guess #12158 will make this less useful unless the macro changes to mirror the try_finally function defined there.

I was correct: it seems like the fact that closures are now correct(er) has sunk the neatness of this macro. Possible solutions:

• Capture the state explicitly, returning a mutable reference.

  (|| { loop { i += 1; /* ... */ } }).finally(|| println!("i = {}", i));
  // becomes
  let mut i = 0;
  let ptr_to_i = defer!(i, println!("I = {}", i));
  loop {
      *ptr_to_i += 1;
      // ...
  }

  (Probably won't actually work, since I don't think there's a way to get the guard to last long enough, or get the i references inside the defer! code to be rewritten to point to the correct variable.)

• Require the use of Cell (same example as before):

  let mut i = Cell::new(0);
  defer!(println!("i = {}", i.get()));
  loop { 
      i.set(i.get() + 1);
      // ...
  }

[nikomatsakis]

@huonw Yeah, it's unfortunate, though I don't anticipate an easy solution. In the meeting we had assumed the macro would only apply to cases where there is no mutable state shared between the two. Possibly the "call out the mutate state" approach could be possibly made to work, though I expect you'd have to write *i inside the macro (as you noted).

[huonw]

I think this is less valuable now, and is better effected by changes like #12235.

(Also, the queue is huge and I'm not going to update this right now... closing.)