Skip to content
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

Champion "defer statement" #1398

Open
5 tasks
gafter opened this issue Mar 19, 2018 · 191 comments
Open
5 tasks

Champion "defer statement" #1398

gafter opened this issue Mar 19, 2018 · 191 comments

Comments

@gafter
Copy link
Member

gafter commented Mar 19, 2018

See #513

  • Proposal added
  • Discussed in LDM
  • Decision in LDM
  • Finalized (done, rejected, inactive)
  • Spec'ed

This is to be considered alongside #1174 (though we could elect to do one without the other)

@gafter gafter added this to the 8.X candidate milestone Mar 19, 2018
@gafter gafter self-assigned this Mar 19, 2018
@alrz
Copy link
Member

alrz commented Mar 19, 2018

This is to be considered alongside #1174 (though we could elect to do one without the other)

I guess this has been brought up before but just to mention, #1174 + "convention-based Dispose" could address the use cases for a "defer statement" (edit: except that it's not anything like resource cleanup, which I believe, such language construction would encourage).

@HaloFour
Copy link
Contributor

@alrz That would be #93

@gafter
Copy link
Member Author

gafter commented Mar 19, 2018

@alrz #1174 + "convention-based Dispose" doesn't help much as a replacement for defer, as it requires creating an object. defer would not create any runtime garbage.

@scalablecory
Copy link

I feel like, outside of calling Dispose (for which there are better proposals), this would be so rarely used that I can't really make a case for its inclusion in the language.

I also think it's very easy to misuse. Not always the case, but generally if you have more than one cleanup block in your code it's a code smell.

#1174 seems much easier for users to use correctly from the standpoint of separation of concerns.

@alrz
Copy link
Member

alrz commented Mar 19, 2018

One valid use case that I can think of, is to save the current state in traversal of some tree structure and reverting back that state once all children are visited.

Still, since this feature is really easy to abuse, I think as long as we keep it undocumented we're fine.

@gafter
Copy link
Member Author

gafter commented Mar 20, 2018

In the spirit of designing the language for myself, there are hundreds of places in the compiler where this would be a simplification of existing code.

@yaakov-h
Copy link
Member

@gafter wouldn't "convention-based Dispose" plus a struct similarly not create an object?

@gafter
Copy link
Member Author

gafter commented Mar 20, 2018

@yaakov-h Sure you can do that. And you're going to store a delegate in the struct I presume so you can pass a lambda with your arbitrary code in it?

@yaakov-h
Copy link
Member

@gafter #302? 😄

@gafter
Copy link
Member Author

gafter commented Mar 20, 2018

Static delegates cannot capture anything

@tannergooding
Copy link
Member

@gafter, static delegates by themselves don't capture anything (as proposed).

This does not, however, mean the compiler could not create some struct which allowed state to be carried and passed to a static delegate when invoked (such as to support some other feature).

@tannergooding
Copy link
Member

(a user could of course do the same with static delegates)

@ygc369
Copy link

ygc369 commented Mar 20, 2018

@gafter
I like this idea. It could help people avoid to forget must-be-done things.
But I wonder whether this would work when "goto" exists.
For example:

{
        SomeType thing = Whatever...;
        defer {
            thing.Free();
        }
        if (SomeCondition) goto outside; //would "thing" be freed before goto outside?
        //some code 
}
outside:
    //some code

@svick
Copy link
Contributor

svick commented Mar 20, 2018

@ygc369 Doesn't the same question exist with finally today? And the answer is that the finally block is executed when you use goto.

@Opiumtm
Copy link

Opiumtm commented Mar 20, 2018

It's still unclear how it would be implemented?
Every defer block could be translated by compiler into separate try..finally, or all deffered blocks could be in the single finally? If all would be in separate finally blocks then OK as it would help to avoid multiple nested try...finally in some cases. Then it's unclear in which exact order deffered blocks could be executed.

@svick
Copy link
Contributor

svick commented Mar 20, 2018

@gafter

In the spirit of designing the language for myself, there are hundreds of places in the compiler where this would be a simplification of existing code.

I wonder if it's more common need in the compiler than in other code. And since compiler writers are not really a target demographic of C#, I think their needs are not that important. "Designing for myself" can work, but I think you need to make sure you're not designing just for yourself.

Static delegates cannot capture anything

Sure, but the struct that's being disposed can.

So I think you could do something like:

using scoped Defer(thing, static t => t.Free());

Or even (assuming using was changed to support ref structs #93):

using scoped Defer(ref thing, static (ref SomeType t) => t.Free());
It's much more verbose than real `defer`, but maybe it will turn out that it's good enough? If not, `defer` could be added in a later version.

@HaloFour
Copy link
Contributor

@gafter

I'd love to hear more about these use cases you envision for defer. I know that you've mentioned temporarily swapping field values and reverting at the end of a scope.

I'm not terribly concerned about how the @gafter's of the world might use this feature. It's everyone else.

Also, assuming that defer is scoped to the current block, I'm putting $500 on an immediate demand for the ability to expand to an outer block or the scope of the method. I bet people will find it frustrating trying to juggle scopes to get their deferred actions to execute when they actually want it to.

@gafter
Copy link
Member Author

gafter commented Mar 20, 2018

@HaloFour I assume you're not asking me.

@HaloFour
Copy link
Contributor

@gafter

I assume you're not asking me.

Yes, the question is in response to your statement which I should've quoted:

In the spirit of designing the language for myself, there are hundreds of places in the compiler where this would be a simplification of existing code.

I'm not trying to challenge your statement, I just want to understand the value-add where existing syntax doesn't suffice for one reason or another.

@Thaina
Copy link

Thaina commented Mar 23, 2018

@gafter

"convention-based Dispose" doesn't help much as a replacement for defer, as it requires creating an object. defer would not create any runtime garbage.

I am support your whole idea and mechanism. But I still thinking that we should not need defer and just use using as of implicit using and solve this problem by compiler and new syntax

Because mechanism-wise, this is actually the using and dispose

As I was propose #249 that we should able to create anonymous local function. In this case we should be able to create local function for mocking the Dispose

As for the simplest case

// Just stranspile in the same way as defer
var thing = Whatever...;
using ~> { // syntax for defer
    thing.Free();
}

And for inline case

// Just stranspile in the same way as defer
using var thing = Whatever... ~> { // syntax for defer
    thing.Free();
};

Also I was once propose dotnet/roslyn#16958 about return using. It would be good if this syntax would be generalized to be disposable object that could be returned

@TheUnlocked
Copy link

TheUnlocked commented Mar 23, 2018

Just going with @Thaina's syntax,

using var thing = new ThingObj(arg, anotherArg) ~> thing.Free();

could be a possible one-liner. Although in this case, ThingObj should really be properly disposable.

@gafter
Copy link
Member Author

gafter commented Mar 23, 2018

As I was propose #249 that we should able to create anonymous local function. In this case we should be able to create local function for mocking the Dispose

The point is to avoid creating unnecessary garbage. Not even a delegate. This should be as efficient as other local control-flow.

@scalablecory
Copy link

If we have implicitly-scoped using, it seems to me that the rest of defer's need could be accomplished with a set of tricky Roslyn optimizations. This'd likely have a double-whammy effect of making the simplest (and probably most common) use of local functions more efficient.

@Thaina
Copy link

Thaina commented Mar 24, 2018

@gafter That's why I try and try to propose #249 that we should always have a way to create anonymous local function that was not create unnecessary garbage. Not even a delegate. And you can see most people just don't care about garbage and want it to be the same syntax as delegate

And as I said it should be compiler optimization to just know that If there are using in from of the line and ~> befire the block then don't create anything but just skip that logic to the end of the function

We don't need defer to achieve your goal just a context sensitive compiler. As @scalablecory said it just need a set of tricky Roslyn optimizations.

@gafter
Copy link
Member Author

gafter commented Mar 24, 2018

@Thaina As long as the language-defined semantics do not require the creation of a delegate or the lifting of variables into closure classes, we don't need any "optimizations" to implement defer. That is true even if we select a syntax for it that uses a sequence of tokens like using ~>. In that case it has nothing to do with IDisposable or Dispose or convention-based anything.

@Thaina
Copy link

Thaina commented Mar 24, 2018

@gafter I actually have a little difference opinion about this. I was actually think about using and ~> as a separate syntax, not a sequence token

I was thinking of using still being the syntax for calling disposal logic at the end of block. But I propose that ~> (could be any alternative syntax) is a syntax for creating disposal logic block separately from using. We then just implement compiler optimization trick to optimize it and call it at the end of function if it was in front of using

Generally we could just always make inline optimization for using with Dispose if we know that it was not a virtual function of the object we put in the using

It would be that from the start we might allow ~> only after using but eventually I wish that it could be a syntax for creating disposable logic on their own. As of my dotnet/roslyn#16958

@dmitriyse
Copy link

dmitriyse commented Dec 13, 2019

What about filtering of exception in a defer block ?

var transaction = new SomeTransaction();
using(transaction)
{
    defer (Exception? ex) => if (ex != null) { transaction.Commit(); }

    // ...
    // Use transaction
    // ...

    throw new InvalidOperationException("Some reason");
}

@lcsondes
Copy link

I like this proposal but I feel we might be able to reuse finally for this instead of adding a new keyword. Having a free-standing finally currently is a syntax error so the syntax could be something like this without affecting any existing code:

var x = Blah();
finally
{
    Console.WriteLine("Cleaning up x");
    x.Cleanup();
}

The only case that I see where this would overlap with existing syntax (and to be clear I'm proposing that try-catch-finally should remain unchanged and work exactly like it does today) is if you wanted to express this:

try {...}
catch(Exception) {...}
defer {...}

Not sure if this has much value (defer right after getting out of the try-catch, it could, dare I say should be before the try since it can access nothing from the scopes of the try-catch anyway?) and it can be resolved with either being explicit with the second level try-finally (as this would be written today in C# <=8) or adding anything, even a lone semicolon before the defer block (that I'm proposing to call finally instead). I don't think this one contrived scenario is worth the extra keyword.

@Zastai
Copy link

Zastai commented Dec 27, 2019

I like this proposal but I feel we might be able to reuse finally for this instead of adding a new keyword. Having a free-standing finally currently is a syntax error so the syntax could be something like this without affecting any existing code:

var x = Blah();
finally
{
    Console.WriteLine("Cleaning up x");
    x.Cleanup();
}

All the nope. In simple code snippets this works. But in real world code you would see

  <several lines of code>
}
finally {
  <code>
}

and have no idea what is actually happening without checking whether or not that } belonged to a try.

I already dislike the proposal because it makes code harder to understand, but reusing finally would make it even worse.

@Grauenwolf
Copy link

Grauenwolf commented Dec 30, 2019

I really don't understand why we want a backwards finally block. But if you really do need it, here it is:

using new Deferable( () => {} );

The only reason we don't do this now is that it's much harder to read than finally blocks. But if you really want it, you have it now. Use it for a few years to prove a special keyword is justified.

@Richiban
Copy link

@Grauenwolf It's already been discussed why using a lambda/wrapper isn't acceptable. For the performance-sensitive scenarios even pattern-based dispose wouldn't be enough.

@TheBuzzSaw
Copy link

@Grauenwolf Is it really any worse than the invisible call to Dispose in a normal using block? It's just nice to be able to keep this logic together.

var handle = new Widget();
defer handle.CleanupMethodWhichIsNotDispose();

@lcsondes
Copy link

This conversation sounds similar to the case of finally in C++: on one hand you have the "if you need finally, your class is not designed well, it should be RAII" people, and then there's the workaround of making a dummy object with a destructor to achieve defer/finally-like behavior.

@Grauenwolf
Copy link

Performance? Do you have any metrics that prove this would really be an issue?

This is already a pretty obscure feature that is better handled by using in the vast majority of cases. Now you want to double-down and claim that these incredibly rare cases where defer might make sense are also cases where you're in the middle of a tight loop where the performance cost of a delegate call is significant?

That doesn't seem plausible to me.

So I'm going to reiterate my challenge. Show us some read-world code employing using new Deferable( () => {} ); with timings that demonstrate performance would be unacceptably slow.

@TheBuzzSaw
Copy link

@Grauenwolf I'm happy to work on a benchmark to at least demonstrate what the difference would be, but I don't think performance (by itself) is necessary to justify the feature. Delegates open the door to captures or other mistakes. Why is defer so evil for having code run "out of order" while using new Deferable(...) is totally off the hook? It has the same problems (which many of us don't see as problems).

Regarding performance, it's just a straight fact that delegates would be slower with pointless allocations. Who cares whether it's a make-or-break difference for any given application? Why not expose faster paths for those who want it?

@Grauenwolf
Copy link

Why is defer so evil for having code run "out of order" while using new Deferable(...) is totally off the hook? It has the same problems (which many of us don't see as problems).

They do have the same problems, hence my challenge.

If you can prove that using new Deferable(...) is consistently useful in production code despite the (non-performance) concerns, then we have a case for defer. If you can't, then we have a good case against defer.

Either way it will settle the debate with code rather than words, which I think we can all agree is preferable.

@HaloFour
Copy link
Contributor

HaloFour commented Dec 31, 2019

@TheBuzzSaw

Regarding performance, it's just a straight fact that delegates would be slower with pointless allocations. Who cares whether it's a make-or-break difference for any given application? Why not expose faster paths for those who want it?

In my opinion, it's more about the fact that defer largely duplicates using. I'd rather see those benefits come to using rather than adding a completely new language feature. A new language feature that duplicates an existing language feature but is less restrictive is effectively a deprecation of the existing language feature, like lambdas were to anonymous delegates.

I'm also concerned that a carte blanche feature like defer encourages its use in scenarios unrelated to resource disposal. I'm not saying that all such uses are abuse, but even in languages that have defer the prevailing opinion seems to be that it should be limited for resource disposal.

Lastly, I have a lot of questions as to when and how defer works. The behavior proposed here already differs from that of Go and D in that it applies to the current block and not the current method. That could be a source of confusion. I could definitely be nitpicking there, though.

@TheBuzzSaw
Copy link

@HaloFour That is largely the point. I want to Wait() on my semaphore and then Release() it no matter how I exit this scope. The defer keyword is more of an attack on finally than it is on using. I would expect to continue using using all over the place.

The Go form of defer is the absolute worst. I don't want that version of defer anywhere near C#. I'm unfamiliar with D's approach.

@HaloFour
Copy link
Contributor

@TheBuzzSaw

I want to Wait() on my semaphore and then Release() it no matter how I exit this scope.

Shame Semaphore doesn't support using, that would solve that problem nicely. defer will encourage more APIs to not support using because there'd be no reason to.

SharpLab

@Grauenwolf
Copy link

I'm also concerned that a carte blanche feature like defer encourages its use in scenarios unrelated to resource disposal.

That's kinda the point. If the scenario is resource disposal then you should just use IDisposable. This only makes sense for situations where you need to unwind something that isn't resource-related.

@ygc369
Copy link

ygc369 commented Feb 24, 2020

@gafter
I support the idea of "defer", but I think that it should be more powerful.
Besides using "defer" directly in methods, we should also create a special kind of struct ----defer struct, to wrap resources.
for example:

defer struct FileManager 
{
    FileStream? file;
    public FileManager(string filename)
    {
        file = new FileStream(filename, FileMode.Open);
    }
    defer // the defer block in a defer struct is automatically executed once the struct instance goes out of scope.
    {
        if(file)
        {
           file.Close();
           file=null;
        }
    }
}

We should introduce the concept of defer struct, it is a special kind of struct, only used for wrapping resources, it should have following features:
(1) can only be declared in methods. (stack-only)
(2) can't be copied. (assignment is only allowed when initialization. Can't assign one defer struct to another.)
(3) can't be method parameter (can't be passed to other methods, no matter by value or by ref).
(4) can't be a field of class or normal struct. But may be a field of another defer struct.
(5) when an instance of the defer struct goes out of its scope, the defer block would be executed at once, so the resources which the defer struct wraps would be freed.

Why to introduce defer struct?
I want to bind defer to resource. If we can only use defer directly in methods, then everytime we use a resource, we must write its defer block too. If the same kind of resource is used in many places (many methods), then we must write the same/similar defer block many times, it might easily be forgotten, and then cause resource leak.
But if we could have defer struct, then the defer block is binded to the resource, when we want to use the resource, we could just use the defer struct that wraps it, not having to remembering to write defer any more! This would make defer more powerful.

@lcsondes
Copy link

That's basically RAII lite. Other than it being done automatically (which to my knowledge is deliberately not done in C# compared to, e.g., C++/CLI, happy to be corrected), what benefits does this bring over Dispose() and using that same struct (think of ref structs in particular)? On the flip side, this does introduce the issue that old C# code will not know how to properly clean up after new types using this feature, however IDisposable is widespread and handled pretty much everywhere.

@Grauenwolf
Copy link

The deferable struct sounds interesting, but it would only make sense if we had a matching resource that could only be stack-allocated.

By the same token, if we had a resource that could only be stack-allocated, we couldn't safely use IDisposable and would need a deferable struct.

But the elephant in the room is whether or not such a resource exists.

@lcsondes
Copy link

@Grauenwolf I thought C# already supported that scenario? using works with ref structs that have a Dispose() on them even through they're incapable of implementing IDisposable.

@Grauenwolf
Copy link

In my imagination, this deferable struct would be recognized by the compiler so it wouldn't need an explicit using statement. But that may not be backwards compatible with existing ref structs.

@Zastai
Copy link

Zastai commented Feb 24, 2020

It’s essentially asking for destructors in a struct. If the CLR adds those, fine, use them for RAII if you like.
But it’s an inelegant way of doing things; in the FileStream case, you would have to repeat all constructor signatures in the wrapper. I don’t see why using var is so bad for this that it needs improving. Yes, you might forget the using but chances are your IDE will warn about undisposed local disposables. So I see no real benefit to these “defer structs”.

@ygc369
Copy link

ygc369 commented Feb 25, 2020

@Grauenwolf

In my imagination, this deferable struct would be recognized by the compiler so it wouldn't need an explicit using statement.

Yes.

But that may not be backwards compatible with existing ref structs.

Add a limitation: deferable struct can't be declared as ref struct.

it would only make sense if we had a matching resource that could only be stack-allocated.

The deferable struct itself must be stack-allocated, but the resource it wraps don't need to be stack-allocated. In my example, the resource FileStream is obviously not stack-allocated.

@MadsTorgersen MadsTorgersen modified the milestones: 9.0 candidate, 10.0 candidate Apr 22, 2020
@MadsTorgersen MadsTorgersen modified the milestones: 10.0 candidate, Likely Never Sep 9, 2020
@sgf
Copy link

sgf commented Nov 10, 2021

i know C# has its selfs ways,but its long syntax.
so,why not let it more shortly !
i voted this idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests