Deconstruct IDisposable in using.

[darren-clark]

Similar to, but may be in conflict with:
#1898
#1473

If I have a disposable class:

public class DisposableDeconstructible: IDisposable
{
   public void Deconstruct(out int x, out int y)
}

Deconstructing this object on assignment will cause loss of reference to the IDisposable object itself.

In this case it seems a statement such as:

using(var (x,y) = GetDisposableDeconstructible())
{
...
}

would hold an unnamed reference to the IDisposable.
Much like:

using (new Disposable())
{
...
}

does not require a variable assignment.

[tpetrina]

Well, writing it manually is only slightly longer...But is it too annoying to warrant a language change you decide.

using (var d = GetDisposable())
{
    var (x, y) = d;
}

Your first proposal hides a bit too much for my taste.

[darren-clark]

I'm not exactly sure you see as being hidden
Another way to write it could have been

using (GetDisposable().Deconstruct(out var x, out var y))
{
}

Which actually works, if you change the return value of Deconstruct(this making it NOT work for actual deconstruction). This obviously has an unnamed local assigned in order to dispose of it.

I also find the actual error message odd, saying "type used in using statement must be implicitly convertible to IDisposable". What type? Are we constructing a tuple from the Deconstruct call, assigning that tuple to an unnamed local and then deconstructing it into variable?

IMHO, the only variables we're declaring in that using statement are x and y. Leaving the only logical type to be "used" as the result of GetDisposable(). There are no(or should not be) any tuples involved in a statement var (x,y) = foo where foo is not a tuple, but has a Deconstruct method.

[canton7]

I don't see the use-case here.

IDisposable objects are closely tied to the concept of ownership. An IDisposable object should have a single owner, and that owner is the thing which is responsible for calling Dispose at the appropriate time. Ownership can be transferred, e.g. passing a Stream to a StreamReader, but then the original owner loses the ownership, and should not call Dispose.

If you deconstruct an IDisposable object, then you're breaking it apart. Ownership of the resource which needs disposing should surely then be transferred to one of the deconstructed parts (or perhaps more than one of them, if there is more than one underlying resource which needs disposing). Either way, ownership has transferred away from the original IDisposable object which was deconstructed, which shouldn't now be disposed.

This goes against your proposal, which explicitly breaks apart the IDisposable object, and then later disposes it.

Do you have a concrete use-case in mind?

[mattwar]

The lesson here is that deconstructable types should not implement IDisposable.

[canton7]

I suspect there are probably cases where deconstructable IDisposable types make sense. I just can't think of a case where ownership shouldn't pass to one of the deconstructed parts.

[darren-clark]

I would agree that in most cases ownership could pass to deconstructed parts. However that isn't something that is normally done. In the case where the object is not deconstructed, what do you dispose? The object? A property on the object?

My use case is that I have multiple instances of a database, all with the same schema. I also have another database that includes information about those databases. I then have a class that provides access to a database as well as a description.

Simplified

public class DBInstance: IDisposable
{
    public Guid Id { get;} 
    public string InstanceName { get; }
    public IDbConnection Connection { get; }
    public void Deconstruct(out string instanceName, out IDbConnection connection); 
}

So we have

using(var (name,connection) = GetInstanceById(<id>))
{
   connection.CreateCommand()
   ....
}

or

using (var instance = GetInstanceById(<id>))
{
   instance.Connection.CreateCommand()
   ...    
}

or with deconstruction

using (var instance = GetInstanceById(<id>))
{
   var (name, connection) = instance;
   connection.CreateCommand()
   ...    
}

I agree that it's not really necessary syntactically, but it reads well to me. I certainly shouldn't be calling Dispose on instance.Connection. I don't own that, and that shouldn't change just because of deconstruction IMO.

[canton7]

I'd argue that once you've deconstructed it into the name and connection, you've taken ownership of the connection, and it's your responsibility to dispose the connection.

If you were just grabbing some properties and putting them in locals, I'd write simply:

var (name, connection) = (instance.Name, instance.Connection);

The deconstruction to me is something more: itvs breaking apart the original and storing its component parts in locals.

Your proposed syntax makes it look like both the name and the connection should be disposed by the using statement to me, when in fact its neither: it's the temporary object which I can't see.

[darren-clark]

I would disagree that deconstructing implies I'm now responsible.

That would mean I need to know the implementation of Dispose on DbInstance . What if it actually doesn't own the connection?

Does that mean

 var instance = GetInstance();
 var (name, connection) = instance;

vs.

 var (name, connection) = GetInstance();

I should call Dispose on different objects, because I made an assignment in the first case? Or should I call Dispose on connection in both cases because I deconstructed DBInstance?

What about

var instance = GetInstance();
var (name,_) = instance;

In the case of direct deconstruction we all know there was an object in a temporary, so I don't see why it's hiding anything. To me my proposed syntax is clear, I understand it isn't to everyone. That was actually what I typed and was a bit surprised that it didn't compile.

I think "passing responsibility for disposal" is dangerous, and something that wouldn't be expected if you were doing variable assignment without the syntactic sugar of deconstruction.

[canton7]

That would mean I need to know the implementation of Dispose on DbInstance . What if it actually doesn't own the connection?

Then DbInstance shouldn't be IDisposable, or its Dispose method shouldn't dispose the connection. Then this whole problem goes away.

I should call Dispose on different objects, because I made an assignment in the first case? Or should I call Dispose on connection in both cases because I deconstructed DBInstance?

I'd argue that you should call Dispose on connection in both cases. However as we've found the situation is very ambiguous, so I'd try to avoid mixing deconstruction and IDisposable to start with!

In the case of direct deconstruction we all know there was an object in a temporary, so I don't see why it's hiding anything.

No we don't. GetInstance might return a ValueTuple. If I saw var (name, connection) = GetInstance() I would assume that you're returning and then deconstructing a ValueTuple: I would not for a minute think that there's an intermediate custom object unless I went digging.

As I said earlier, that's another reason why I think this is confusing. using (var (name, connection) = GetInstance()) definitely looks like GetInstance returns a tuple, and I'm left wondering "well what on earth is being disposed?". The only natural conclusion to someone unfamiliar with the syntax is that this is some shorthand for disposing both name and connection, which is very much not what is happening!

Manual resource management is hard, and it's easy to get wrong, so there's IMO a lot to be said for keeping things clear and understandable.

[mattwar]

This is really not the use case for deconstruction. Deconstruction is meant for types that behave like tuples. They are data-only class (without behavior other than maybe equality, etc). They should not own any aspect of disposing of resources.