Champion: extensions in pattern-based statements (16.3, Core 3)

[jcouv]

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

Scenarios:

• using statements and declarations, and foreach should find Dispose() extension methods (on ref structs)
• await using statements and declarations, and await foreach should find DisposeAsync() extension methods (on any type)
• foreach statements should find GetEnumerator() extension methods on ref structs
• await foreach statements should find GetAsyncEnumerator() extension methods (on any type)

We should review other pattern-based statements

Being pattern-based means:

1. Binding “normally”, but only instance members
2. Going through the interface
3. Trying to bind extension methods

FYI @gafter

Relates to

• Proposal: Allow using statement to structurally match IDisposable on ref structs (16.3, Core 3) #1623 (enhanced-using)
• Champion "Async Streams" (including async disposable) (16.3, Core 3) #43 (async-streams)

[bbarry]

Does this cover Dispose extension methods where the parameter uses ref or in?

[HaloFour]

As mentioned in #2133 (comment) could we consider supporting pattern-based disposal for using for types that are not ref structs? I understand that the source breaking changes concerns were specifically regarding foreach, not using.

[jcouv]

Thanks @bbarry @HaloFour Those are good questions. I'm hoping LDM will discuss later this week.

[jcouv]

From discussion in LDM today, extensions should not participate in pattern-based disposal. The main scenario that seemed interesting is to allow GetEnumerator() extension methods to contribute.

[HaloFour]

@jcouv

That's a shame. Allowing extensions to participate in pattern-based disposal (as well as permitting pattern-based disposal for using statements) would enable one to write easy adapters to dispose arbitrary types without requiring completely new and redundant language features.

[YairHalberstadt]

Given that there's hopes to add a more formal definition of an extension type in a future major release (C# 9 ?), would it be sensible to avoid making too many changes now which would be rendered unnecessary in the future?

[jcouv]

@YairHalberstadt There is a championed issue for extension everything (#192) and another one for adding an interface after-the-fact (#110).

[YairHalberstadt]

As well as the roles shapes and extensions one created by Mads

[HaloFour]

It was noticed that using pattern-based Dispose on a ref struct passed by ref will cause a copy of the struct, is this intentional?

public void M(ref Foo foo) {
    using (foo) { 
    }
}

results in:

    .method public hidebysig 
        instance void M (
            valuetype Foo& foo
        ) cil managed 
    {
        // Method begins at RVA 0x2054
        // Code size 18 (0x12)
        .maxstack 1
        .locals init (
            [0] valuetype Foo
        )

        IL_0000: ldarg.1
        IL_0001: ldobj Foo  // creates copy of the struct
        IL_0006: stloc.0
        .try
        {
            IL_0007: leave.s IL_0011
        } // end .try
        finally
        {
            // sequence point: hidden
            IL_0009: ldloca.s 0
            IL_000b: call instance void Foo::Dispose()
            IL_0010: endfinally
        } // end handler

        IL_0011: ret
    } // end of method C::M

[CyrusNajmabadi]

It was noticed that using pattern-based Dispose on a ref struct passed by ref will cause a copy of the struct, is this intentional?

From my reading of hte spec (which is admittedly light here), i think this is due to https://github.com/dotnet/csharplang/blob/master/spec/statements.md#the-using-statement:

When ResourceType is a non-nullable value type, the expansion is

{
    ResourceType resource = expression;
    try {
        statement;
    }
    finally {
        ...

So even if you just have an identifier name in using (foo) and foo is by-ref, that expansion still applies, and we'll operate on the local temp instead.

[CyrusNajmabadi]

Now: there's an open question of if we want that behavior. And, if we do, if we want some way for someone to say "can you please not copy these structs? i'm using 'ref' for a reason, and htis is basically defeating the purpose there".

[HaloFour]

@CyrusNajmabadi

Makes sense. Seems it's to guard against reassignment of the variable as the warning when you do reassign does explicitly state that Dispose will be called on the original value:

warning CS0728: Possibly incorrect assignment to local 'foo' which is the argument to a using or lock statement. The Dispose call or unlocking will happen on the original value of the local.

Given that the compiler is detecting this reassignment it would be nice if it only had to make the defensive copy in the case that the variable is reassigned.