Champion: Implicitly scoped using statement (16.3, Core 3)

[HaloFour]

See dotnet/roslyn#5881 and dotnet/roslyn#181

Motivation (TL;DR edition):

Allow using keyword to be used to declare a variable that will be disposed at the end of the current block. This avoids creating a new block with an additional level of indentation:

Before:

public void Foo() {
    using (var connection = new SqlConnection(connectionString)) {
        connection.Open();
        using (var command = connection.CreateCommand()) {
            command.CommandText = "SELECT FOO FROM BAR";
            using (var reader = command.ExecuteReader()) {
                while (reader.Read()) {
                    ProcessRecord(reader);
                }
            }
        }
    }
}

After:

public void Foo() {
    using var connection = new SqlConnection(connectionString));
    connection.Open();
    using var command = connection.CreateCommand());
    command.CommandText = "SELECT FOO FROM BAR";
    using var reader = command.ExecuteReader());
    while (reader.Read()) {
        ProcessRecord(reader);
    }
}

As an implementation detail, the variables would be disposed in the reverse order in which they are declared. So given the above code, the order in which the resources would be disposed is reader, command and then connection.

[scottdorman]

I think this has a potential to introduce a lot of unintended consequences and unexpected behavior. How is this any better than the "before" code, except I didn't need to type 2 extra sets of parens and 2 extra sets of curly braces?

Using already creates an implicit single-line block, in much the same way as a single line if statement. Dropping the curly braces has too much risk to introduce bugs in the code, in much the same way a single line if statement without curly braces can. (Adding what you think is a second statement in the if block, but because there aren't curly braces it's actually not part of the if block even if it's indented.)

Disposability is non-deterministic and there isn't anything the compiler can do to tell the runtime GC that things should be disposed of in a certain order.

[HaloFour]

How is this any better than the "before" code, except I didn't need to type 2 extra sets of parens and 2 extra sets of curly braces?

And extra indentation. When your code doesn't even start until half-way off of the screen it's a big waste of real-estate.

Disposability is non-deterministic and there isn't anything the compiler can do to tell the runtime GC that things should be disposed of in a certain order.

This has nothing to do with GC. It is strictly syntax candy for using blocks, and it is strictly deterministic. The reverse declaration disposal order is the expected behavior.

[jnm2]

I'll copy my comments from dotnet/roslyn#16579:

This is very easy to confuse visually (I intentionally indented it wrong):

using var stream1 = new MemoryStream();
    string v1 = new StreamReader(stream1).ReadToEnd();

using (var stream1 = new MemoryStream())
    string v1 = new StreamReader(stream1).ReadToEnd();

Even worse, here the indentation is fine:

using var stream1 = new MemoryStream(); // Oops, stream1 will live on
using (var deflate = new DeflateStream(stream1, CompressionMode.Decompress))
    string v1 = new StreamReader(deflate).ReadToEnd();

using (var stream1 = new MemoryStream())
using (var deflate = new DeflateStream(stream1, CompressionMode.Decompress))
    string v1 = new StreamReader(deflate).ReadToEnd();

On the humorous side, since you aren't requiring parentheses, this could be legal:

private static ILolz System;

void Lolz()
{
    using System.Text;
    // ...
}

[scottdorman]

And extra indentation. When your code doesn't even start until half-way off of the screen it's a big waste of real-estate.

I'd argue that if you have so many nested using statements (or other nested code blocks) where that's a problem then you need to refactor into simpler/smaller methods. That point aside, I still don't see this giving us a real benefit but introduces a lot of places which will lead to incorrect code. (See @jnm2's earlier comment. All of the issues mentioned are legitimate concerns, are harder to read, and can introduce even harder to find runtime bugs.

This has nothing to do with GC. It is strictly syntax candy for using blocks, and it is strictly deterministic. The reverse declaration disposal order is the expected behavior.

It does have to do with GC as the using statement has to do with GC since it's expanded by the compiler to a call to IDisposable.Dispose(). Since GC is nondeterministic, we have no control over the order which the GC will decide to dispose of objects once they're out of scope so you can't say that this is deterministic, especially if you're saying that it's merely syntax sugar for using blocks.

[jnm2]

@scottdorman @HaloFour's proposal still has the disposable.Dispose() call deterministically, but now it's invisible and as late as possible without the disposable instance going out of scope. GC is not involved because it's always disposed in the scope it was created.

[scottdorman]

@jnm2 Ok. I think I understand the meaning here now. It's deterministic in the sense that this code

public void Foo() {
    using var connection = new SqlConnection(connectionString));
    connection.Open();
    using var command = connection.CreateCommand());
    command.CommandText = "SELECT FOO FROM BAR";
    using var reader = command.ExecuteReader());
    while (reader.Read()) {
        ProcessRecord(reader);
    }
}

will (roughly) expand to this code:

public void Foo() {
   var connection = new SqlConnection(connectionString));
   try {
      connection.Open();
      var command = connection.CreateCommand();
      try {
         command.CommandText = "SELECT FOO FROM BAR";
         var reader = command.ExecuteReader();
         try {
            while (reader.Read()) {
	       ProcessRecord(reader);
            }
         }
         finally {
            ((IDisposable)reader)?.Dispose();
         }
      }
      finally {
         ((IDisposable)command)?.Dispose();
      }
   }
   finally {
      ((IDisposable)connection)?.Dispose();
   }
}

Which means the order of the calls to Dispose() are in the same order. But that's it, the GC doesn't really care what order they're called in and isn't guaranteed to release the resources in that order. Just because you call Dispose() does not mean the GC has done any work yet.

[jnm2]

@scottdorman I don't like this syntax sugar at all, but the GC is not affected by this proposal. If you manually type the braces for the using statements, they way we work today, the effect on the GC is identical.

[scottdorman]

@jnm2 The motivation in the initial post says

So given the above code, the order in which the resources would be disposed is reader, command and then connection.

Maybe it's just the way I read it, but it specificly defines an order in which the resources are expected to be disposed, which isn't possible.

Other than that, you're right, this proposal theoretically doesn't change the compiler generated expansion of the using statements, so the resulting code should be identical to what is produced today.

I'm still not a fan of this syntax as I think it has a huge potential to introduce hard to find/debug runtime errors and decreases the code readability.

[jnm2]

But you do have that guaranteed order of resource release in both scenarios! The GC doesn't release resources like connections, commands and readers. Dispose does that, deterministically, right when you tell it to. All GC does is free managed memory when it can be proved that freeing the memory has no effect on the executing code. Dispose and GC honestly have almost nothing to do with each other.

[Opiumtm]

@jnm2

Dispose and GC honestly have almost nothing to do with each other.

Not completely right.

Finalizers are called by GC and most holders of unmanaged resources do implement finalizer. SafeHandle, for example, is freed via finalizer by GC if don't freed explicitly already. When IDisposable.Dispose() is called, finalizer is suppressed.

There are two drawbacks of finalizers (and it's why they're not used in most real-life scenarios and IDisposable.Dispose() is used instead).

1. No guaranteed order of execution
2. Relatively big cost of finalizers as they require more work on GC thread and can cause noticeable performance degradation

[HaloFour]

@scottdorman

Yes, this proposed syntax is intended to behave (and expand) exactly the same as using blocks do today, except that instead of specifying a separate nested block that the compiler uses the remainder of the current block. The "Before" and "After" code blocks in the proposal are intended to behave exactly the same. This is strictly compiler candy dealing with calling Dispose on disposables.

As for excessive nesting, the example above is real world and fairly common. It's three extra levels of nesting just for the sake of defining those blocks. In some cases it is possible to avoid that nesting. You can declare multiple resources in the same using statement, but that requires that they all be of the same type. Or you can use another using statement as the embedded statement of another using statement. But, as in the example above, you have to invoke any code on the resource before defining another resource, it's currently not possible to avoid the nesting. And refactoring code as simple as such above would require significantly more work, especially since you'd have to return something representing a composite disposable but still exposes the same contract as the original resource. This proposal simplifies all of that.

I'll give @jnm2 the concern that the syntax can be confusing. Afterall, the difference is only in parenthesis. Of the proposals on the Roslyn repo this was the syntax that @gafter seemed to prefer, so that's why I re-proposed it here. I'd be willing to explore other syntax.

[scottdorman]

@jnm2

Dispose and GC honestly have almost nothing to do with each other.

They absolutely have everything to do with each other. The only reason IDisposable existed in the first place was to provide us a way to manually release unmanaged resources as early as possible.

When IDisposable.Dispose() is called, finalizer is suppressed.

Only if it's written properly, which many are not.

SafeHandle was created well after .NET released partly because there were so many incorrect implementations when dealing with IntPtrs. The SafeHandle derived classes are meant to wrap IntPtrs so the dispose pattern is implemented properly without relying on the developer to "get it right".

Dispose does that, deterministically, right when you tell it to. All GC does is free managed memory when it can be proved that freeing the memory has no effect on the executing code.

GC deals with both managed and unmanaged resources and does so by calling Dispose(bool) and/or the finalizer. It doesn't know how to release memory, only how to figure out if there is still a live reference to an object (and it really only does that work itself as a last resort and instead asks various parts of the runtime, like the JIT, for what's still alive). The actual mechanism of releasing any resources is done so in the context of the Dispose(bool) method and/or finalizer.

[scottdorman]

@HaloFour Yes, I realize the code example you gave is very real-world and have written many such blocks myself but have never felt it burdensome. To be honest, if we're going to introduce syntax sugar around dispose, I'd much rather see syntax sugar introduced that makes it easier/trivial to write the pattern properly (the IDisposable.Dispose() method is correct, calls to the base class Dispose` method are chained in properly, etc.) than syntax sugar that reduces the amount of indention and curly braces I have to write.

That is, in my opinion, completely trivial when compared with ensuring the pattern is correct. A great example of this is a strongly typed WCF client, which does not follow the pattern properly and cannot be used inside a using statement as it will throw an exception when the scope is closed.

This proposal reduces the amount of indenting and curly braces I have to write, but introduces a much greater risk for invalid code due to the syntax. Given the syntax between the two referenced proposals, this syntax is better, but I still disagree that this feature is valuable.

[mikedn]

using var stream1 = new MemoryStream(); // Oops, stream1 will live on

So what? In most cases this is at best a performance issue because the object may be disposed later than it could be. It may sometimes be a correctness issue if you're trying to reuse a resource that you think you released earlier but in fact you did not:

using Stream s1 = File.Open(...);
s1.Write...
using Stream s2 = File.Open(...); // tough luck, this may fail due to sharing violation
s2.Write...

But such code is rare in my experience.

SafeHandle was created well after .NET released partly because there were so many incorrect implementations when dealing with IntPtrs.

Sort of. It would be more correct to say that it was created because it is practically impossible to deal with IntPtr safely. But what does this has to do with this issue? This is not about how GC works, it's about some rather trivial syntactic sugar.

I'm mostly "for" this proposal but that may be due my C++ background where this is basically the "default" syntax.

That said, I think that the most important drawback of this proposal is that it introduces an alternative way to do things. Such alternatives have already been introduced in the language but usually the new way is better and becomes the default while the old way becomes irrelevant (e.g. anonymous methods vs. lambda expressions).

[HaloFour]

@scottdorman

Yes, I realize the code example you gave is very real-world and have written many such blocks myself but have never felt it burdensome.

I'll let you borrow my Surface Pro 2 for a while. It was my primary development machine for a couple of years and it served that purpose very well. Not everyone has really wide monitors. 😄

To be honest, if we're going to introduce syntax sugar around dispose, I'd much rather see syntax sugar introduced that makes it easier/trivial to write the pattern properly

Well, unless this becomes an either/or proposition and it's impossible for the team to consider both separately, there's no harm in proposing it.

I completely acknowledge that this proposal is very minor in the grand scheme of things. I didn't even originally propose it, either on CodePlex or on Github. I just liked it enough, and knew that it at least had a little nod from members on the LDM team, that I thought it deserved to be brought over here.

[scottdorman]

@HaloFour

I'll let you borrow my Surface Pro 2 for a while. It was my primary development machine for a couple of years and it served that purpose very well. Not everyone has really wide monitors.

Fair enough. Screen size constraints are a real thing. I have a 3 monitor arrangement on my main development system, but definitely feel the loss of productivity when I work on my single-screen 15" laptop.

Well, unless this becomes an either/or proposition and it's impossible for the team to consider both separately, there's no harm in proposing it. I completely acknowledge that this proposal is very minor in the grand scheme of things. I didn't even originally propose it, either on CodePlex or on Github. I just liked it enough, and knew that it at least had a little nod from members on the LDM team, that I thought it deserved to be brought over here.

Absolutely. We would all be doing ourselves (and the language team) a disservice if we didn't bring up a proposal and weren't able to have discussions about it. That's the whole point here. Just because somebody (me, in this instance) may disagree with a proposal, doesn't mean it shouldn't still be discussed.

[sharwell]

@scottdorman It sounds like you are confusing IDisposable.Dispose with finalization in some of your comments above. Consider the simple case of using a FileStream to write to a file. In this case, there are two resources of interest:

1. The handle to the open file provided by the operating system
2. The instance of FileStream being used by the application

While each of these resources needs to be freed at some point, the timing of the first is particularly important - until the OS handle is closed, a user will not be able to open Explorer and delete the file. To ensure resources like this are only kept open while they are used, the IDisposable interface (and by extension the using statement) allows the user to explicitly release the operating system handle after the file is no longer being written. The second resource, which is the FileStream instance itself, doesn't have observable side effects like the OS handle, so it's simply left for the garbage collector to reclaim at some point in the future. There are two important observations here:

1. User code is solely responsible for releasing unmanaged resources, such as the operating system handle from the example. (Sometimes this is implemented in the base class libraries, but the point is it's not part of the GC.)
2. The GC is responsible for reclaiming the memory allocated to instances of managed objects which are not longer in use, such as the FileStream instance from the example.

The using statement is a language construct which allows users to perform the operation they are responsible for in a deterministic manner. Finalizers exist as a fallback to avoid memory leaks over time due to misuse of disposable objects (failure to call Dispose, followed by the object no longer being used). However, finalizers incur substantial overhead for tracking and have extremely difficult-to-understand semantics, so they should only be implemented on small objects whose sole purpose is exposing the unmanaged resource to managed code. Since the .NET Framework (and now .NET Standard) provides two very flexible base classes for these wrappers - SafeHandle and CriticalHandle - we are now at a place where user-defined finalizers should essentially disappear from application code. In the end the only part we really need to deal with is IDisposable.Dispose and figuring out when to call it ourselves (with a using statement where possible). In this way I think what @HaloFour mentioned regarding using not being about the GC makes sense. Even in the total absence of a GC the using statement could be used to release unmanaged resources such as OS handles.

I cannot recommend the following highly enough: http://joeduffyblog.com/2005/04/08/dg-update-dispose-finalization-and-resource-management/

[iam3yal]

Reducing the level of nesting is a great thing when it comes to make code legible and the syntax is almost the same so why not? :)

Reiterating on what @MadsTorgersen wrote:

• Stick to the topic of the issue title. If a comment is tangential, start a new issue and link back.
• If a comment goes into detail on a subtopic, also consider starting a new issue and linking back.

So let's not change the subject here, I mean it's about adding an alternative syntax so derailing the discussion into GC and/or resource management is really not the place to do it.

[sharwell]

I lean in favor of this proposal, but perhaps not for the reasons mentioned previously. One example where I think this could be useful is in recording telemetry around actions invoked by a user. In these situations, the telemetry recording which wraps the "real" work of a method can often be implemented using a using statement, but it isn't really a resource and the method implementation typically wouldn't interact with it. For example:

// BEFORE
public async Task UserOperationAsync()
{
  using (Telemetry.Operation(nameof(RunOperationAsync)))
  {
    // work here
  }
}

// AFTER
public async Task UserOperationAsync()
{
  using Telemetry.Operation(nameof(RunOperationAsync));

  // work here
}

⚠️ My primary point of concern at this point is the ambiguity with parenthesized expressions. If the new form of using statement was defined as the following:

scoped_using_statement
  : 'using' resource_acquisition ';'
  ;

🐉 Then the resource_acquisition here could actually be a parenthesized_expression, and you end up with a valid using_statement with totally different semantics in the current language.

📝 I would consider this proposal relevant/related to #85, but while the scenarios overlap I wouldn't say either fully encompasses the other.

[iam3yal]

I don't know but how do you feel about the following?

var connection = using new SqlConnection(connectionString));

[HaloFour]

@eyalsk

My only concerns there would be that it implies that the behavior is detached from the declaration, which is a big departure from the current behavior where the variable is declared at that point and is readonly. Would that assignment be legal to an existing variable? Could the variable be reassigned?

Those differences might not be blockers, though. The compiler could still ensure that the instance itself is disposed, but there might be a question as to which scope that instance would be attached to. That of the variable, or that of where the using expression is evaluated:

public void Foo() {
    SqlConnection connection = null;
    if (condition) {
        connection = using new SqlConnection(connectionString);
        // do stuff here
    }
    // do other stuff here
}

// equivalent to

public void Foo() {
    SqlConnection connection = null;
    if (condition) {
        using (var $temp = new SqlConnection(connectionString)) {
            connection = $temp;
            // do stuff here
        }
    }
    // do other stuff here
}

// or equivalent to

public void Foo() {
    SqlConnection connection = null;
    try {
        if (condition) {
            connection = new SqlConnection(connectionString);
            // do stuff here
        }
        // do other stuff here
    }
    finally {
        (connection as IDisposable)?.Dispose();
    }
}

Personally I'd prefer the former. It might be weird that the variable lifetime and scope outlives that of the resource, and attempting to use it afterwards could result in exceptions. The compiler could potentially warn about that. It's no different than assigning off a reference to that instance anyway, which you've always been able to do.

Furthermore, should the compiler care if you assign that instance to fields or ref/out parameters? Probably not, for the same reason above.

[iam3yal]

@HaloFour

Would that assignment be legal to an existing variable? Could the variable be reassigned?

Excellent point.

The reason I suggested it is because people raised the concern where it looks too similar to the current statement and it might be confusing but yeah I fully agree with your point.

[scottdorman]

@sharwell Fair enough (some of those comments may not have been as clear as they could have been). Yes, with the use of SafeHandle and CriticalHandle it's almost never necessary to implement a finalizer anymore.

@eyalsk Agreed. The discussion about GC is off-topic to this proposal. Sorry for the temporary derailment.

@sharwell's point about this being more akin to a scoped unit of work type scenario is valid and I'd still much rather see something around that than a different syntax to simply reduce the amount of indenting and which can introduce correctness problems.

[Thaina]

@jnm2 Copied from my proposal too

First, for

using (var stream1 = new MemoryStream())
string v1 = new StreamReader(stream1).ReadToEnd();

There are error Embedded statement cannot be a declaration or labeled statement

Second, for

using (var stream1 = new MemoryStream())
string v1 = new StreamReader(stream1).ReadToEnd();

using (var stream1 = new MemoryStream())
using var deflate = new DeflateStream(stream1, CompressionMode.Decompress);
    string v1 = new StreamReader(deflate).ReadToEnd();

It will become obvious if you let formatter do indent

using (var stream1 = new MemoryStream())
    string v1 = new StreamReader(stream1).ReadToEnd();

using (var stream1 = new MemoryStream())
    using var deflate = new DeflateStream(stream1, CompressionMode.Decompress);
string v1 = new StreamReader(deflate).ReadToEnd();

And Embedded statement cannot be a declaration or labeled statement
Will pop out too

[Thaina]

We could avoid reassignment by forcing using var cannot be reassign. That would be most obvious solution

[Thaina]

If there are condition need to met I would use ternary operator

public void Foo()
{
    using var connection = condition ? new SqlConnection(connectionString) : null;
    if (connection != null) {
        // do stuff here
    }
    // do other stuff here
}

[jnm2]

@Thaina like I said before: the embedded statement error doesn't matter. Pretend I removed the word var and my original point still stands.

Like I said before: the formatter does not indent like you say. Because there are no braces, both using statements stay at the same level of indent. This is the style used everywhere I've ever seen. So my original point here still stands as well.

[Thaina]

@jnm2 And like I was response to you that if you don't assign variable in that line then it would not cause any problem. It will only be a problem when you try to use any variable

using (var stream1 = new MemoryStream())
    new StreamReader(stream1).ReadToEnd();
// vs
using var stream1 = new MemoryStream();
    new StreamReader(stream1).ReadToEnd();

What would be difference? it only that you would have stream1 in difference scope and just that

[HaloFour]

@CyrusNajmabadi

That doesn't look deeply nested to me :)

Add telemetry and a couple of resources which can't be all instantiated back to back, plus namespace, class and method indentation and you're already 20+ spaces over. If you don't think that's a lot then I'll let you borrow my 10" Surface Pro 2 which was my primary development machine for about 4 years.

Note: i would not want defer to cause allocations.

It wouldn't have to if you guys allowed for Dispose by convention like you do with so many other language features. Then you could have value disposables just like you have value enumerators. But you guys enjoy being inconsistent. 😉

Every language that has a defer feature recommends it specifically and explicitly for resource cleanup. It is considered bad practice to use it for anything else. C# has a mechanism for resource cleanup.

[CyrusNajmabadi]

But you guys enjoy being inconsistent.

I don't see why teh snark is necessary. It's not like people are going out of their way to do this. This is a language that has been around a long time and which always has work being done with it continuously. That means that not every issue can be addressed. Would you like it if your customers treated every rough edge in any product you made as being maliciously intentional? :-/

It wouldn't have to if you guys allowed for Dispose by convention like you do with so many other language features

I don't see how that would help here. The Deferable type had an private Action _action. Delegates cause allocations today. So you'd have to pay that price. I would prefer an alloc free way to have this feature, and i don't see how to do that without language support.

[yaakov-h]

Wasn't there a proposal somewhere for zero-allocation delegates?

[HaloFour]

I don't see why teh snark is necessary.

More poking fun. Let me add some emoji in there to better convey my intent.

Would you like it if your customers treated every rough edge in any product you made as being maliciously intentional?

Wait, you mean that they sometimes don't? 😜

I don't see how that would help here. ... I would prefer an alloc free way to have this feature, and i don't see how to do that without language support.

Maybe using by convention over a local method?

I'd probably be more amenable to the idea of defer if it had non-pathological use cases outside of those already covered by using. I've yet to see a single example in another language that didn't, aside from randomly printing stuff out of order just to demonstrate how it works. That's a lot of general-purpose syntax rope to handle an narrow slice of problems not already easily handled.

[yaakov-h]

@HaloFour

Maybe using by convention over a local method?

Is that not "language support"?

[HaloFour]

@yaakov-h

Is that not "language support"?

Of course it is. So is adding defer. Just throwing some spaghetti at ways the same goals can be accomplished today without helpers that cause allocations like Disposable.Create(() => { }).

[lucasteles]

I would like to omit the var keyword, i think this can be more clear and not confuse with the current syntax

public void Foo() {
    using connection = new SqlConnection(connectionString));
    connection.Open();
    using command = connection.CreateCommand());
    command.CommandText = "SELECT FOO FROM BAR";
    using reader = command.ExecuteReader());
    while (reader.Read()) {
        ProcessRecord(reader);
    }
}

[qwertie]

There is a good reason to require parentheses as @jnm2 noted: using System; looks like it should be an import, but it wouldn't be.

In EC# there is a pattern where you can write things that ordinarily require braces without braces, causing them to treat the entire rest of the block as their subject:

• At the top level, namespace Foo; puts the rest of the file in a namespace
• { replace (WL => Console.WriteLine); etc; etc; } means { replace (WL => Console.WriteLine) {etc; etc;} }

So naturally I recommend that if this proposal is implemented, it should be implemented simply by allowing the braces to be omitted.

From my perspective as author of EC#, I find the ambiguity of this proposal horrifying, because EC# uses a single interpretation mode for statements at all nesting levels (that is, you can write expressions like Foo() at the top level of a file and you can write imports like using System.Text inside a method).

[CyrusNajmabadi]

So naturally I recommend that if this proposal is implemented, it should be implemented simply by allowing the braces to be omitted.

Braces can already be omitted with using statements today :) And htat already has a very specific meaning in the language. :)

[gafter]

The championed tracking issue for this feature is #1174 .

[xen2]

Also throwing my vote in favor.
Another advantage I can think of is, VCS diff will be reduced quite a lot.
It often happens that adding a using around a bunch of code makes it much more verbose to review due to the indent diff.

[theunrepentantgeek]

@xen2 this is already available in the latest Visual Studio 2019 preview, so you can download and try it out today.

I'd also suggest you find a better diff tool, one that doesn't get confused by whitespace changes. I'm a big fan of Beyond Compare, very much worth the price.

[kimsey0]

Shouldn't this issue be closed, since the change landed i C# 8.0?

[333fred]

We don't close issues until the specification update is merged into the language. We have a draft speclet in the proposals folder, but nothing in the language itself yet.