Proposal: change the semantics of a using declaration combined with an object initializer

[jeroen-mostert]

Consider the following:

class MyDisposable : IDisposable {
    readonly IDisposable x;

    public MyDisposable() {
        x = new ...;
    }

    public int I {
        get => 0;
        set => throw new NotImplementedException();
    }

    public void Dispose() {
        x?.Dispose();
    }
}

using var myDisposable = new MyDisposable {
    I = 0
};

The compiler will currently translate that into the moral equivalent of:

MyDisposable myDisposable = new MyDisposable();
myDisposable.I = 0;
var myDisposable2 = myDisposable;
try {
} finally {
    myDisposable2?.Dispose();
}

Which exposes the original code as having a bug: if the initializer throws, the Dispose is never called, though it really ought to be. This matches the semantics of using (var myDisposable = new MyDisposable { I = 0 }), which analyzers have warned about for a while now (the infamous CA2000).

The problem is that the new using declaration syntax is a really inviting target for refactoring what would appear to be "verbose" initialization:

using var myDisposable = new MyDisposable();
myDisposable.I = 0;

In fact, public versions of VS offer to refactor this to initializer syntax, which under the current semantics is a bug.

But instead of fixing this bug and sharpening the analyzers to give more meaningful messages about this, is there any objection to digging a pit of success and making the semantics of this statement match what would be the actual intent of the developer in almost all cases? That is, to have using var x = new C { X = ... , Y = ... } be translated to the equivalent of var x = new C(); using (x) { x.X = ..; x.Y = ..; }?

The cons are obvious: the semantics would not match the existing using block semantics closely anymore, which is potentially confusing (and harder for the compiler to get correct), and it is not backwards compatible, meaning that any code bases that actually expect the Dispose to not be called when using an object initializer that throws would break -- but I expect real-world code that relies on this to be much rarer than currently incorrect code that would want the opposite.

Importantly, I do not propose to change the semantics of using (var x = new C { X = ... }), because this pattern, even though also potentially broken, has been in code bases for much longer, and the impact of changing the semantics there are harder to gauge. using var has not been in the language for long enough to be entrenched like this, and is also much more visually inviting to initializer refactoring, even though under current rules it's not safe.

[HaloFour]

Importantly, I do not propose to change the semantics of using (var x = new C { X = ... })

I think it's worth exploring. Even if it has the potential to change the semantics of more existing code, I think that change is a positive one and will more likely correct an existing problem than cause regressions.

[jeroen-mostert]

I actually agree , but wanted to stress that even if it was decided that changing the semantics of that is too daring, it should not block changing the semantics of the declaration syntax to maintain a foolish consistency.

[ronnygunawan]

Doesn't this Implement interface with Dispose pattern solve the problem? You can uncomment the generated finalizer and call x?.Dispose(); after // TODO: free unmanaged resources (unmanaged objects) and override a finalizer below.

[alexrp]

@ronnygunawan It's not necessarily appropriate for every kind of disposable object to have a finalizer. It makes sense for objects that just own unmanaged state, but people have come to use IDisposable for many other things over the years because it's convenient (similar to defer in Go, scope (exit) in D, etc). For these cases, failure to dispose is usually a programmer error that could leave the application as a whole in an unpredictable state, so trying to make up for it with a finalizer that gets called at some unspecified time in the future from a background thread can range from "not great" to "really bad".

[ronnygunawan]

@alexrp fair enough.

I also want to add a simpler case:

class MyDisposable : IDisposable {
    readonly IDisposable x;

    public MyDisposable(object someArg) {
        x = new ...;
        // if (someArg is null)
            throw new ArgumentNullException();
    }

    public void Dispose() {
        x?.Dispose();
    }
}

The bug can still be reproduced here without object initializer.

[jeroen-mostert]

@ronnygunawan That's not a common scenario -- this is why people do things like argument checking before running any non-trivial code in the method, especially a constructor. A more realistic scenario would be this:

class MyDisposable : IDisposable {
    readonly IDisposable x;
    readonly IDisposable y;    

    public MyDisposable() {
        x = new ...;
        y = MyComplexDisposable.Create(...);  // might throw, leaving x undisposed
    }

But the purpose of this proposal is not to eliminate all possible problems with partially initialized objects leaving things undisposed (that's impossible, unfortunately) but to make an intuitive pattern for initialization safe without the developer having to take special precautions, and without analyzers hassling you that you're doing something wrong, just because of the way the compiler chose to implement things.

[wizedave]

@ronnygunawan, errors and exceptions during execution of the constructor are responsibility of that object, I don't think those should be caught by the using's implicit try-finally block.
It is the Constructor's responsibility to release partially allocated unmanaged resources before letting the exception propagate out; unlike object initializers, which are, in fact, executed after an object was successfully created.

[poke]

To provide an example that’s outside of constructor failures:

void Main()
{
    using var foo = new Foo
    {
        Prop = GetValue(),
    };
}

public int GetValue() => throw new ArgumentException("derp");

public class Foo : IDisposable
{
    public int Prop { get; set; }
    public Foo() => Console.WriteLine("Foo");
    public void Dispose() => Console.WriteLine("Foo disposed");
}

Here, the GetValue throws during property initialization, so foo was already successfully constructed. But the exception will cancel the execution before the using statement had an effect so foo is never disposed.

[isxaker]

similar topic is being discussed here as well
https://stackoverflow.com/questions/66473229/why-dispose-is-not-called-in-case-of-using-block-and-object-initializer?noredirect=1#comment117514498_66473229