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

Proposal: 'using' local variable declaration (RAII) #5881

Closed
rhencke opened this issue Oct 12, 2015 · 85 comments
Closed

Proposal: 'using' local variable declaration (RAII) #5881

rhencke opened this issue Oct 12, 2015 · 85 comments

Comments

@rhencke
Copy link

rhencke commented Oct 12, 2015

Motivation

In C# (and VB), using statements contain an embedded statement (traditionally, a block statement) that contains the code that will be run before the resource is disposed.

When multiple resources require disposal, using statements are nested to accomplish this (though traditionally written at the same level of indentation)

using(var r = new Resource()) 
using(var s = new OtherResource())
{
}

This works well if all resources can be acquired at the same time, but this cannot always be done. The motivating example, in this case, is the traditional boilerplate for ADO.NET, in which the block scopes provide more noise than signal.

void foo()
{
    using (var conn = new SqlConnection("..."))
    using (var cmd = conn.CreateCommand())
    {
        connection.Open();

        using (var reader = cmd.ExecuteReader())
        {
            while (reader.Read())
            {
                // ...
            }
        }
    }
}

Proposal

For scenarios like this, explicitly providing block scope often feels like overkill. While it's important to dispose of the resources properly, the block scope containing the using is often sufficient enough.

I would like to propose the following syntax:
using local-variable-declaration ;

This would effectively be syntactic sugar for 'Place this local variable declaration in a using statement, and place all statements within the same block that follow this statement into the statement list of that using block'. Basically, the desired effect is the variable follows normal scoping rules, and as soon as the variable falls out of scope, the resource is disposed.

With this new syntax, we can now write the above example as:

void foo()
{
    using var conn = new SqlConnection("...");
    using var cmd = conn.CreateCommand();

    connection.Open();

    using var reader = cmd.ExecuteReader();

    while (reader.Read())
    {
        // ...
    }
}
@paulomorgado
Copy link

Do you really think this is worth all the work involved?

@pawchen
Copy link
Contributor

pawchen commented Oct 12, 2015

Nice, I like this

@HaloFour
Copy link

This has been proposed before, but I can't find the issue so perhaps it was over on CodePlex. Either way, I also like this.

Combining using with var does seem a little weird since the variable would have to be assigned and then would remain readonly for the remainder of the method. Perhaps with val/let depending on how that proposal shakes out.

@pawchen
Copy link
Contributor

pawchen commented Oct 12, 2015

The using keyword is naturally associated to IDisposable, while val feels more like struct, and let sounds immutable to me.

var is just a placeholder for type, despite being somewhat unfortunately/mistakenly felt like a variable.

So unless better keyword proposed, I would prefer using for this time.

@HaloFour
Copy link

The using keyword would be involved either way. let or val are being weighed as keywords for readonly locals, so the immutable connotation is appropriate given that you're not permitted to reassign a using variable today nor should you be able to in this proposal.

@rhencke
Copy link
Author

rhencke commented Oct 12, 2015

paulomorgado: Personally, yes, I do. I have seen many cases where having explicit control over the Dispose point through a new block scope is unnecessary, and variable scope would have sufficed. I understand every feature starts at -100. I'm willing to try to make my case.

@rhencke
Copy link
Author

rhencke commented Oct 12, 2015

HaloFour: I agree the silent read-only nature of the variable assignment is a bit unfortunate, but I came to the same conclusion - it's a practice established already through the current using statement semantics, so my hope is that it could be justified as such.

@rhencke
Copy link
Author

rhencke commented Oct 12, 2015

Also - yes, to be clear, the variable assignment would be read-only. This is intended to have exactly the same semantics as the traditional 'using' statement, but just in the current block scope.

@HaloFour
Copy link

Here's the CodePlex proposal for reference: 544789

@rhencke
Copy link
Author

rhencke commented Oct 12, 2015

HaloFour: Thanks for the reference. I believe this proposal is identical to that one, yes. It looks like it never reached a conclusion there? If I'm reading that right, this could be considered the GitHub issue for it, then.

@whoisj
Copy link

whoisj commented Oct 12, 2015

Oh the wonders if let implied using when the underlying type is IDisposible.

void foo()
{
    let conn = new SqlConnection("...");
    let cmd = conn.CreateCommand();

    connection.Open();

    let reader = cmd.ExecuteReader();

    while (reader.Read())
    {
        // ...
    }

    // conn, cmd, and reader are disposed here
}

@bkoelman
Copy link
Contributor

How would the proposed syntax work out in case the IDisposable instance is not assigned to a variable? Since there is no variable that goes out of scope, at what moment will Dispose be called?

using (new RuntimeDurationLogger(Console.Out))
{
    // slow code
}

@rhencke
Copy link
Author

rhencke commented Oct 12, 2015

bkoelman: I should have been clearer here - this proposal only permits the form of the using construct where there is a variable declaration.

This is not intended as a replacement for the traditional using construct - merely a complement to it.

@HaloFour
Copy link

Or there could be a using <expression>; form. There would be an implicit variable anyway (as there is today) and it would be disposed where it would have gone out of scope:

if (condition) {
    using new RuntimeDurationLogger(Console.Out);
    // do stuff here
    // Dispose called here
}

@rhencke
Copy link
Author

rhencke commented Oct 12, 2015

HaloFour: This certainly could be done. I had omitted this support from the proposal because I could not think of a scenario needing it, and it felt strange to have code that might just say 'using myVar;' somewhere. But, it would simplify the proposal to do what you are saying here.

@whoisj
Copy link

whoisj commented Oct 12, 2015

@HaloFour in the use-case you describe above, when do you envision the variable leaving scope and thus being disposed?

Personally, I would assume the using would dispose the object as soon as the method/constructor (new RuntimeDurationLogger in your example) returned.

@HaloFour
Copy link

@whoisj

the use-case you describe above, when do you envision the variable leaving scope and thus being disposed?

At the closing brace of the if block. The behavior would be the same as if the expression were assigned to a variable. It would effectively convert into the following:

if (condition) {
    using (new RuntimeDurationLogger(Console.Out)) {
        // do stuff here
    }
}

Personally, I would assume the using would dispose the object as soon as the method/constructor (new RuntimeDurationLogger in your example) returned.

Wouldn't be much point in that, would there? Might as well just write new RuntimeDurationLogger(Console.Out).Dispose();.

@whoisj
Copy link

whoisj commented Oct 12, 2015

@HaloFour

What is the point of waiting for the closing of the if block? I do not see how that is explicit in any way.

I was working off the assumption that using new RuntimeDurationLogger(Console.Out); was equivalent to using (new RuntimeDurationLogger(Console.Out)) ;. I do not think that I would be alone in that camp and thus this could cause much confusion.

@HaloFour
Copy link

@whoisj Because keeping the reference alive/undisposed within the scope is the purpose of that class? That's why using blocks don't require variable declarations today. I don't think it's confusing, but that may be reason enough to avoid such a syntax. Variable scope is certainly easier to piggyback.

@pawchen
Copy link
Contributor

pawchen commented Oct 12, 2015

@HaloFour

let is already a keyword in Linq, also readonly, but is not constrained/specially tweaked to work with IDisposable, there are other proposals about immutable so maybe leave this keyword to them?

I think this proposal is about simplifying, making less indents, it's better keep it simple.

Well the different behavior between using ( blah ); and using blah; surprise me though, but I wonder if anyone would create something and dispose immediately.

@rhencke
Copy link
Author

rhencke commented Oct 12, 2015

For what its worth:

  • using (var x = new X()) ; emits a compiler warning today.
  • using var x = new X() { /* ... */ } would be a syntax error in this proposal.

@HaloFour
Copy link

@DiryBoy I was referring to proposal #115 to introduce readonly parameters and variables in which they explicitly mention a var-like shorthand for inferred local variables using either let or val, although I do believe they were leaning towards val. My mention has nothing to do with IDisposable rather the fact that these variables would have to be readonly.

@pawchen
Copy link
Contributor

pawchen commented Oct 12, 2015

As mentioned by @rhencke the following code today already generates CS1656

using ( var d = new Disposable() )
{
    d = new Disposable();
}

So no need to emphases readonly in a simplified version IMO.

@pawchen
Copy link
Contributor

pawchen commented Oct 12, 2015

OK, @HaloFour I read your first comment again and understand what you meant, it's about combination, using var .. and using val/let .. are both good.

@bondsbw
Copy link

bondsbw commented Oct 12, 2015

We could just get rid of the confusion altogether:

using conn = new SqlConnection("...");
using cmd = conn.CreateCommand();

That would really just shorthand for whatever syntax is agreed upon (using var vs. using val/let), which would still be legal if writing it out explicitly is preferred.

I'd like to see removing the requirement for var in more cases where declaration is the only possibility. For example:

foreach (result in results) { ... }

@mburbea
Copy link

mburbea commented Oct 12, 2015

There is still be a good reason to allow declaring type as the class may have explicitly declared interface methods, or shadowed members that can only be called when cast to the appropriate type.

There is also some coding styles which prefer avoiding using var at all cost. It's really a mixed bag thing, and I wouldn't want to force people to change their styles in the name of sugar.

As for the the using(SomeMethodThatReturnsIDisposable()){}, it's not completely insane. Some times, the creating and disposing is all you want. Especially in the case of classes that abuse the using .

@paulomorgado
Copy link

@whoisj

Oh the wonders if let implied using when the underlying type is IDisposible .

void foo()
{
    let conn = new SqlConnection("...");
    let cmd = conn.CreateCommand();

    connection.Open();

    let reader = cmd.ExecuteReader();

    while (reader.Read())
    {
        // ...
    }

    // conn, cmd, and reader are disposed here
}

What if I have a SqlConnection factory method? I light of what's being proposed for let as readonly var, this would be valid:

SqlConnection GetConnection(...)
{
    let connection = mew SqlConnection(...):
    // ...
    return connection;
}

Under your suggestion, it would be returning a disposed connection.

@paulomorgado
Copy link

@rhencke, some say -100 others say -1000.

What would you drop from what the team is proposing to work on in favor of this?

@rhencke
Copy link
Author

rhencke commented Oct 12, 2015

@paulomorgado: This was a reference to http://blogs.msdn.com/b/ericgu/archive/2004/01/12/57985.aspx.

Quite simply, I am not the right person to make decisions in that area. If you have objections to the proposal itself, I am happy to discuss them with you.

@gafter gafter reopened this Aug 15, 2016
@gafter
Copy link
Member

gafter commented Aug 15, 2016

Since we like this syntax better than #181, we'll track it using this issue.

@whoisj
Copy link

whoisj commented Aug 23, 2016

Ya know, all of this comes down to C# having terrible ownership semantics. The introduction of a "borrowed" reference would likely resolve most of these issues.

(I now return you to your originally scheduled topic about using disposables)

@lixiaoqiang
Copy link

just like deffer in golang, very good

@VALLIS-NERIA
Copy link

I wish "fixed" statement could works similarly.

fixed char* p1=&str1[0];
fixed char* p2=Foo(str2);
// blahblah...

@Porges
Copy link

Porges commented Jul 17, 2017

Is there a separate proposal for doing this at class scope?

e.g. have this expand to have an automatically-implemented Dispose method

class Example : IDisposable
{
    using Stream _stream;
    public Example(string path) {
        _stream = File.OpenRead(path);
    }
}

@Thaina
Copy link

Thaina commented Jul 17, 2017

@Porges It was mentioned in other issue sometimes. I don't remember. But the main problem there is it must be readonly or else it might be replaced with another instance and the old one would not be disposed

@Porges
Copy link

Porges commented Jul 17, 2017

@Thaina, good point. Perhaps if it were restricted to readonly fields.

@whoisj
Copy link

whoisj commented Jul 18, 2017

Should we be keeping the related discussion here: dotnet/csharplang#114 ?

@tannergooding
Copy link
Member

It would also be useful if fixed statements worked in a similar manner (implicitly scoped to the containing block).

@whoisj
Copy link

whoisj commented Sep 1, 2017

It would also be useful if fixed statements worked in a similar manner (implicitly scoped to the containing block).

Um...

fixed (char* pchar = m_value)
{
    // do things with pointer.
}

Not sure I want the member m_value being destroyed at the end of that fixed statement.

@tannergooding
Copy link
Member

@whoisj, I meant more of the perspective of being implicitly scoped, rather than being RAII. stackalloc, at least based on the underlying IL instruction and the C# spec, does not allow explicit freeing of the memory.

The using statement itself is essentially RAII already (lifetime starts at the beginning of the scope and ends at the closing of the scope), so this request is essentially just stating that the explicit scope declaration should be considered unnecessary and we should support it being implicitly determined by the compiler (this mostly just removes levels of nesting and makes code easier to read).

The fixed statement doesn't really have anything to do with RAII, but does have the same issue that using has, which this proposal is suggesting we fix (explicitly declaring blocks is tedious and makes code harder to read).

@whoisj
Copy link

whoisj commented Sep 1, 2017

@tannergooding sounds to me, what you're really after is Rust-like ownership semantics. Me too, but it would predicate too many changes in the CLR to happen in any reasonable time frame.

@alrz
Copy link
Member

alrz commented Sep 1, 2017

@whoisj I think @tannergooding is talking about something like this:

{
  fixed int* p = e;
  // use p
}

which is equivalent to:

{
  fixed (int* p = e)
  {
    // use p 
  }
}

@tannergooding
Copy link
Member

@alrz gets it, I'm just bad at communicating sometimes 😄

@whoisj
Copy link

whoisj commented Sep 1, 2017

I'm just bad at communicating sometimes

It is just as likely that I'm just dense and not understanding, but yup I get it now thanks to @alrz 😄

@hoensr
Copy link

hoensr commented Jul 27, 2018

This is what I was looking for today.
My code is something like this:

List<MyResults> results;
using (var sheet = new MyExcelSheet(excelFilePath))
{
    results = myExcelReader.ReadResults(sheet);
}

I would like to be able to shorten it to

var results = myExcelReader.ReadResults(using new MyExcelSheet(excelFilePath));

(MyExcelSheet is a thin facade around EPPlus, implementing an interface so I can unit-test the Excel reader.) Note that in the long code, I need to give the explicit type of the variable results, whereas in the short code, I can use var.

@paulomorgado
Copy link

@hoensr, where would be the start and end of the scope of that object?

@hoensr
Copy link

hoensr commented Jul 27, 2018

Since the MyExcelSheetis an anonymous variable, it would be like in C++: The scope ends at the semicolon.

@paulomorgado
Copy link

I'm asking for the scope of the object. When will it be disposed?

@hoensr
Copy link

hoensr commented Jul 27, 2018

Sorry to be unclear. At the semicolon, too. Anything else would be a surprise, wouldn't it?

@paulomorgado
Copy link

Can you show in what that would be lowered to?

@hoensr
Copy link

hoensr commented Jul 27, 2018

Sorry, I'm not sure I understand what you mean with "lowered to". My idea that these inline using statements are equivalent to a using block ending with the current statement. See the equivalent version in current syntax above.

@HaloFour
Copy link

@hoensr

Discussions regarding language design have moved over to the csharplang repository. This issue is a championed proposal and the conversation has continued here: dotnet/csharplang#1174

@hoensr
Copy link

hoensr commented Jul 27, 2018

@HaloFour : Thank you! I guess I will add my comment over there, then.

@gafter
Copy link
Member

gafter commented May 19, 2019

This is being done in C# 8.0. See dotnet/csharplang#1174 for any further discussion.

@gafter gafter closed this as completed May 19, 2019
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