Skip to content

Update RequestDelegateGenerator to use interceptors compiler feature #48289

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

Closed
captainsafia opened this issue May 17, 2023 · 0 comments · Fixed by #48817
Closed

Update RequestDelegateGenerator to use interceptors compiler feature #48289

captainsafia opened this issue May 17, 2023 · 0 comments · Fixed by #48817
Assignees
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-rdg
Milestone

Comments

@captainsafia
Copy link
Member

captainsafia commented May 17, 2023

The RequestDelegateGenerator currently takes advantage of overload resolution semantics to favor Map overloads that are generated at compile-time over those that are shipped in framework and use runtime code compilation.

The compiler now has an "interceptors" feature which allows generating a method at compile time with an InterceptsLocation attribute that will indicate the generated method should be favored over an in-framework method invoked at a particular file-path, line number, and character.

Note: to improve maintainability of the generator, moving forward all code will be emitted using the interceptors feature and we'll avoid using overload resolution semantics in the code gen. The intercepts feature is more explicit about which overloads are executed given a particular invocation which will make providing user support and debugging for this feature much easier. Going all in on one generation strategy will also reduce the burden of having to produce different kinds of overloads for different scenarios.

The following changes will need to be made to the code generation in reaction to this feature:

1. Generated Map definitions no longer need to be emitted in internal class

At the moment, we emit the compile-time generated Map definitions that are meant to override the in-framework ones into an internal namespace so they can participate in overload resolution.

internal static class GenerateRouteBuilderEndpoints
{
  internal static global::Microsoft.AspNetCore.Builder.RouteHandlerBuilder MapGet(...) { } 
}

With interceptors, these definitions can be emitted into the file private class generated by the compiler which has the benefit of reducing pollution in the user's namespace.

file static class GenerateRouteBuilderEndpoints
{
  [InterceptsLocation]
  internal static global::Microsoft.AspNetCore.Builder.RouteHandlerBuilder MapGet(...) { } 
}

2. Generated Map definition no longer need to define strongly-typed delegate argument

Currently, the compile-time generated Map definitions emit a definition with a delegate parameter that is strong-typed, as seen below, so that the generated overload is favored during overload resolution.

internal static global::Microsoft.AspNetCore.Builder.RouteHandlerBuilder MapGet(
            this global::Microsoft.AspNetCore.Routing.IEndpointRouteBuilder endpoints,
            [global::System.Diagnostics.CodeAnalysis.StringSyntax("Route")] string pattern,
            global::System.Func<global::System.String?[], global::System.Int32> handler,
            [global::System.Runtime.CompilerServices.CallerFilePath] string filePath = "",
            [global::System.Runtime.CompilerServices.CallerLineNumber]int lineNumber = 0) { }

With interceptors, we no longer need to strongly-type the Delegate parameter and do not need to provide the CallerLinePath and CallerLineNumber values into the compiler.

[InterceptsLocation]
internal static global::Microsoft.AspNetCore.Builder.RouteHandlerBuilder MapGet(
            this global::Microsoft.AspNetCore.Routing.IEndpointRouteBuilder endpoints,
            [global::System.Diagnostics.CodeAnalysis.StringSyntax("Route")] string pattern,
            global::System.Delegate handler) { }

3. Emit InterceptsLocationAttribute definition in generated code

The interceptors implementation relies on an implementation of InterceptsLocationAttribute that is not yet defined in the core libraries. For now, this attribute will need to be emitted as a file private type in the generated code:

namespace System.Runtime.CompilerServices
{
    [AttributeUsage(AttributeTargets.Method)]
    internal sealed class InterceptsLocationAttribute : Attribute
    {
        public InterceptsLocationAttribute(string filePath, int line, int column) {}
    }
}

4. Remove reliance on Dictionary thunks

Currently, the RequestDelegate Generator maintains a dictionary that associates a SourceKey that references an endpoint with the MetadataPopulator and RequestDelegateFactoryFunc methods that are associated with it. This look up serves as a cache for endpoints that end up resolving to the same overload but have differing implementations.

The MetadataPopulator and RequestDelegateFactoryFunc can be inlined in the interceptor Map method in this implementation to avoid the cost of dictionary lookup/strategy.

[InterceptsLocation]
internal static global::Microsoft.AspNetCore.Builder.RouteHandlerBuilder MapGet()
{
  MetadataPopulator metadataPopulate = ...;
  RequestDelegateFactoryFunc createRequestDelegate = ...;
  return global::Microsoft.AspNetCore.Http.Generated.GeneratedRouteBuilderExtensionsCore.MapCore(
                endpoints,
                pattern,
                handler,
                GetVerb,
                populateMetadata,
                createRequestDelegate);
}

This also has the benefit of making the code-gen logic much simpler since we have fewer IncrementalValueProviders to concatenate in the end when we construct the argument source.

5. Update logic for casting Delegate into strongly-defined handler inside generated code

The RequestDeelgateGenerator currently casts the generic Delegate provided in the RequestDelegateFactoryFuncwith an explicit cast like so:

var handler = (System.Func<int, string, Todo, IResult>)del;

Due to a breaking change in the inferred type for method groups with lambdas in the new version of the compiler, we need to update the cast to use a delegate that passes through the default value into the conversion.

var handler = Cast(del, global::System.Int32 (global::System.String[] arg0) => throw null!);
private static T Cast<T>(Delegate d, T _) where T : Delegate
{
    return (T)d;
}

A prototype of this implementation can be found at this branch.

@captainsafia captainsafia added feature-rdg area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc labels May 17, 2023
@captainsafia captainsafia added this to the 8.0-preview6 milestone May 17, 2023
@captainsafia captainsafia self-assigned this May 17, 2023
@captainsafia captainsafia changed the title Update RequestDelegateGenerator to use interceptors language feature Update RequestDelegateGenerator to use interceptors compiler feature May 30, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-rdg
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@captainsafia @BrennanConroy and others