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

Extension shadowing #7061

Open
RikkiGibson opened this issue Mar 15, 2023 · 3 comments
Open

Extension shadowing #7061

RikkiGibson opened this issue Mar 15, 2023 · 3 comments
Labels
Proposal Question Question to be discussed in LDM related to a proposal

Comments

@RikkiGibson
Copy link
Contributor

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'?

  • If implicit extension is always a better match than classic extensions, they don't need to move.
    • This is how extensions are currently specified.
  • If classic extensions are equally good match as implicit extensions, they do need to move.

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.

@RikkiGibson RikkiGibson added the Proposal Question Question to be discussed in LDM related to a proposal label Mar 15, 2023
@Mrxx99
Copy link
Contributor

Mrxx99 commented Mar 17, 2023

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

@huoyaoyuan
Copy link
Member

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
Copy link
Contributor Author

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() { }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Question Question to be discussed in LDM related to a proposal
Projects
None yet
Development

No branches or pull requests

3 participants