dotnet / csharplang

The official repo for the design of the C# programming language
11.43k stars 1.02k forks source link

Extension shadowing #7061

Open RikkiGibson opened 1 year ago

RikkiGibson commented 1 year ago

We're interested in more broadly exploring the design space for how "optimizing/specializing" facilities can be added to the platform. In this case we're looking at a language-level solution, but we also want to explore runtime or platform-level solutions. I apologize that the prose isn't as polished here as I was working up something that would be presentable to LDM pretty close up to the time of the meeting.

Related to #7009. Tangentially related to #3992.

Question: What's the use of inheriting from an implicit extension (#5497)? Answer: Shadowing!

// AspNet.Core.dll
namespace AspNet.Core.Routing;

public implicit extension EndpointRouteBuilderExtensions for IEndpointRouteBuilder
{
    public void MapGet(string route, Delegate handler) => ...;
}

// User.generated.cs
using AspNet.Core.Routing;
namespace AspNet.SpecializedRouting;

implicit extension SpecializedRouting : EndpointRouteBuilderExtensions
{
    public void MapGet(
        string route,
        Delegate handler,
        [CallerFilePath] string? filePath = null,
        [CallerLineNumber] int lineNumber = -1,
        [CallerCharacterNumber] int characterNumber = -1) // https://github.com/dotnet/csharplang/issues/3992
    {
        switch (filePath, lineNumber, characterNumber)
        {
            case ("User.main.cs", 5, 11): MapProducts(handler);
            case ("User.main.cs", 6, 11): MapUsers(handler);
            default: base.MapGet(route, handler); // in NativeAOT, you might call a throw helper here instead
        }
    }
}

// User.main.cs
using AspNet.Core.Routing;
using AspNet.SpecializedRouting; // adding this 'using' causes us to prefer extensions from DerivedEx over BaseEx.

IEndpointRouteBuilder app = ...;
app.MapGet("/products/{productId}", (int productId) => ...);
app.MapGet("/users/{user}", (int productId) => ...);

// Problem: delegate conversion might cause us to miss using the derived extension.
// Would need to warn for a usage like this in NativeAOT.
// Is this a fixed limitation? e.g. can the compiler create a *thunk* here to pass along default arguments,
// a bit like what it does to bake-in the receiver for a classic extension method group?
// What would be the impact of a change like that?
Action<string, Delegate> del = app.MapGet; // uses EndpointRouteBuilderExtensions.MapGet

This seems to enable a kind of composition: if you want to do an extra something in an extension, you can extend it, shadow it, and call the base if you need to. You just need to know how to slot yourself into the extension hierarchy.

Problem: ASP.NET uses classic extension methods for MapGet etc. Do they need to move to 'implicit extension'?

Problem: It's somewhat difficult for trimming tools to work optimally with "dispatch blocks" where not all the code paths will get hit at runtime. This is a problem already when trimming the core libraries, e.g., when we dispatch on the StringComparison enum, and although the user's app only uses a limited number of comparison kinds, we end up having to include code for all the comparison kinds.

Question: should extensions be able to shadow methods on their underlying type? Should the underlying method need to give permission for extension to shadow, or can extensions just shadow anything? This seems pretty risky and hopefully unnecessary. We hope that for libraries that want to expose things as extensible in this way, that they can simply write them as members of implicit extensions. However, a scheme that could enable this is described in https://github.com/RikkiGibson/csharplang/blob/interceptors/proposals/extension-shadowing.md.

Question: how do we decide which extensions are "better" in the case of non-linear extension hierarchies? The answer might involve treating more-derived extensions as better, throwing the applicable members from all of them in a method group and running an overload resolution, walking up the extension hierarchies until we find a result (ambiguous or not). This would have to be specified very carefully.

Mrxx99 commented 1 year ago

Would this allow mocking of extension methods? If yes that would be a great improvement over the current extension methods

huoyaoyuan commented 1 year ago

If extension methods are allow to shadow instance methods in some way, can we use this feature to set a preference of GetAwaiter related methods to avoid ConfigureAwait at each call site?

RikkiGibson commented 1 year ago

Would this allow mocking of extension methods? If yes that would be a great improvement over the current extension methods

I think it wouldn't really work for a mocking scenario. The shadowing only occurs statically, so it's not like a mock where you pass it in and virtual calls now go to methods controlled by the test, for example.

If extension methods are allow to shadow instance methods in some way, can we use this feature to set a preference of GetAwaiter related methods to avoid ConfigureAwait at each call site?

You could.

The extension-shadowing proposal as written requires the base method to opt in, so in that case, the platform would have to decide to allow it, and then you could do it.

There's also a possibility of simply letting any extension method shadow any instance method. I think people have an instinctive ick factor about this. Particularly, I think shadowing virtual methods or methods which implement interface members could have surprising results.

var derived = new Derived();
derived.M(); // calls 'Ext.M()'

Base @base = derived;
@base.M(); // dispatches to 'Derived.M()'--'Ext.M()' doesn't get called

class Base
{
    public virtual void M() { }
}

class Derived : Base
{
    public shadowable override void M() { }
}

class Ext : Derived
{
    public new void M() { }
}