[Proposal]: Interceptors

[RikkiGibson]

Interceptors

Feature specification has been moved to https://github.com/dotnet/roslyn/blob/main/docs/features/interceptors.md. As this is not currently implemented as a language feature, we will not be tracking specific status pieces on this issue.

Design meetings

• https://github.com/dotnet/csharplang/blob/main/meetings/2023/LDM-2023-02-27.md
• https://github.com/dotnet/csharplang/blob/main/meetings/2023/LDM-2023-07-12.md#interceptors
• https://github.com/dotnet/csharplang/blob/main/meetings/2023/LDM-2023-07-24.md#interceptors

[CyrusNajmabadi]

Can you spec out the paths, esp vis a vis unix vs windows paths? Specifically, this seems problematic:

File paths used in [InterceptsLocation] must exactly match the paths on the syntax nodes by ordinal comparison

Unless the compiler is already normalizing these paths across platform.

[CyrusNajmabadi]

public static void InterceptorMethod(this C c, int param)

I worry we're doing this at the wrong time. This only seems to be for methods, when we might want it for properties, etc. should this wait for 'extensions' to be done first?

[CyrusNajmabadi]

Constructors are a bit of a sore point for the Regex scenario. It would be nice to be able to intercept new Regex("..."), but it would be strange for an object-creation expression to return an instance of a subtype, for example, even if the expression's static type is unchanged.

I don't see this as any morally wrong versus calling one method and another being called. :)

[CyrusNajmabadi]

takes a qualified method name. For example, for the method bool System.Text.RegularExpressions.Regex.IsMatch(string), we would use the System.Text.RegularExpressions.Regex.IsMatch.

Would prefer that this break up the type-name and member-name into separate pieces.

[HaloFour]

There's a lot to digest here but I'm trying to reconcile how this proposal compares to the AOP scenarios that I often face. At first glance this seems really limited, especially if only one interception can happen at a time. It also seems that the interception cannot "wrap" the target, at least not unless the target is itself accessible from the interceptor. There's no access to the underlying joinpoint, which would make aspects like logging, tracing, transactions, bulkheading, etc. possible. This seems to be very specialized around the specialization of framework code.

I have other concerns about how this is very much spooky action at a distance, as in looking at a PR you can no longer tell if two identical method invocations back to back actually call the same method. There's also the fragility that if the generator doesn't run the interception becomes broken.

[yaakov-h]

To me this feels extremely limited and niche, and at the same time introduces a lot of moving parts that need to be kept in sync.

There also doesn't seem to be a way to opt in or out of it for different invocations, the only construct I can think of would be comments like the /* lang=regex */ syntax highlighting comments, which could be hard to read and understand with sequential invocations like builder objects.

The obvious question I have though is how would this work with default parameters? Would the interceptor need to know the default value and mirror it, or would that be passed through even though the caller did not specify it?

[alrz]

If I read this correctly I think this is basically replace/original on the call-site. There's #3992 to help with that but this doesn't seem to make it easier?

It seems possible to implement a version of replace/original using interceptors

You can already do this (sort of):

[AddLogging]
public partial void Method();
private void Method_Impl() { .. }

I think it comes down to a not-so-hacky way to do either of those, interceptors is definitely an option but still kind of hacky IMO.

The first case is like an overload sensitive to constant arguments. That's pure/constexpr which can also interact with generators:

 // could evaluate to 3 at compile-time, provided all arguments are constant and `Add` is pure.
var result = Add(1, 2);

// could evaluate the argument at compile-time provided it's a constant, using source generators.
var result = Evaluate(expressoin: "1+2"); 
var result = IsMatch(str, pattern: "\d+"); 

// generate a specialized IsMatch for each constant pattern
partial bool IsMatch(string, const string pattern)

The second case is kind of like a horizontal new:

extension for C {
  public new void Method() { .. } // preferred over the original C.Method
}

I've filed dotnet/roslyn#20631 for this a while back - right now it can be silently ignored.

[orthoxerox]

The biggest drawback is probably spooky action at a distance. I understand that by marking my method interceptable I acquiesce to having my logic replaced, but now the user of my library Foo can reference another library Bar that silently generates interceptors for my method that do whatever they want.

[TheUnlocked]

We still think it's important that this works by adding source code to the compilation, and not by exposing source generator APIs that modify the object model of the compilation in any way. It is a goal that you can build a project with source generators, write all the generator outputs to disk, delete the generators, and then build again and have the same behavior.

I don't necessarily agree that the first sentence is the best way to achieve the goal in the second sentence. If there was an API to make direct edits, I would imagine that the compiler could still generate some kind of serialized patch containing those edits which it could save to disk and read later. While that would be pretty fragile in the event of source changes, it wouldn't be any more fragile than referencing a character offset, and it might avoid needing to re-parse/bind everything in the (probably more typical) case when you aren't saving/reading the modifications to/from disk.

[HaloFour]

The more I think about this proposal the less I think it addresses more general-purpose AOP scenarios. I use AOP heavily in my Java services. We routinely use aspects for cross cutting concerns. Those concerns include tracing, logging, metrics, bulk heading, circuit breaking, concurrent call rates, retries (with exponential backoff), memoization, transactions and declarative security. I've also written more specific aspects to facilitate instrumentation of third-party libraries. This interceptor solution may cover some of those cases, but even then it seems it would be a non-ideal way to accomplish it. It doesn't handle other commonly requested scenarios at all, such as IPNC, since the interception happens at the call-site. There's nothing wrong with call-site AOP, but it's only one flavor and can only handle certain use cases. Worse, this proposal explicitly specifies that multiple interceptors aren't legal, but that's an incredibly common case in my code.

I honestly don't see what use cases this proposal is intended to satisfy, except the extraordinarily narrow case of allowing ASP.NET to replace user-written code with their own.

It's also been suggested that this is an appropriate route to implement optimization, where runtime-provided source generators will rewrite some percentage of the user-written code with more runtime-optimized (but certainly less readable) versions. That feels 100% backwards to me, and still very narrow. Not to mention, such "optimizations" can only ever benefit people using newer versions of C# with support for these intercepting generators. Every other language is left out.

Coming from languages where post-processing and byte-code rewriting are a normal thing, this is extremely bizarre to suggest as a language enhancement.

[teo-tsirpanis]

How would [InterceptsLocation] accepting strings as file names interact with dotnet/roslyn#57239 when/if it is implemented? Wouldn't it remove the guarantee that file names are unique?

[RikkiGibson]

Thank you for the feedback so far.

#7009 (comment)

Can you spec out the paths, esp vis a vis unix vs windows paths?

Note that the compiler doesn't require paths to have any particular form. They just have to be representable in a .NET string.

You can really think of this as: imagine the method we are calling has a CallerFilePath parameter. What string would we pass there? The string we put in the InterceptsLocationAttribute needs to exactly match that string in order to intercept that call. If the implicit CallerFilePath string argument differs based on which environment we are compiling in, then the argument to InterceptsLocation needs to change in the exact same way.

This is very problematic for any manually-written InterceptsLocationAttribute--it's automatically non-portable, unless very specific pathmap arguments are used--for example, have no idea if the right thing will just start happening if you pathmap \ to /. Maybe, maybe not. It starts to feel like a huge hack to make it work at all.

I worry we're doing this at the wrong time. This only seems to be for methods, when we might want it for properties, etc. should this wait for 'extensions' to be done first?

Great question. It does feel like if we wanted to do this, it should be possible to do it with pretty much any non-type member, unless we had a specific reason for leaving them out. This tends to raise the question about how to denote the "location" of a usage of any kind of member, in a way that is not ambiguous with the usage of any other member. For example, use of implicit conversion operators might be tough to intercept based on location.

#7009 (comment)
I wasn't sure what you meant by underlying joinpoint.

#7009 (comment)
I wasn't sure what IPNC means.

That feels 100% backwards to me, and still very narrow. Not to mention, such "optimizations" can only ever benefit people using newer versions of C# with support for these intercepting generators. Every other language is left out.

I definitely want us to consider if the use case of the platform specializing calls to platform methods within a project is better served by an post-compilation IL rewriting step. That will require revisiting any reasons we've been reluctant to introduce such a step in the past.

It's clear that disallowing composition of multiple interceptors is really limiting here. As we look a little more broadly at this space I would definitely like for us to consider what it would mean to allow composition. The sequencing definitely seems easiest to define when you also control the original definition of the thing whose calls will get replaced.

#7009 (comment)

There also doesn't seem to be a way to opt in or out of it for different invocations

I think this would require the "syntactic marker at interception site".

The obvious question I have though is how would this work with default parameters? Would the interceptor need to know the default value and mirror it, or would that be passed through even though the caller did not specify it?

The interceptor would not need to know or mirror the default value, but it would need to have a parameter corresponding to that optional parameter--just as we disallow implicitly converting the method group of void M(string x, string y = "a") { } to Action<string>, for example. In fact, it might be bad for interceptors to use default values--since we don't want to bind the original call site any differently, we're likely to want to spec this to not even insert any default values which are specified by the interceptor.

#7009 (comment)

Wouldn't it remove the guarantee that file names are unique?

Although it's hard to have duplicate file names in a real world project, there's no guarantee of unique paths in the compiler itself. (This was a significant complicating factor for file-local types.) Most likely, if a single tuple of (filePath, line, column) does not refer to exactly one source location in the compilation, it needs to be an error to try and intercept the location. This could be problematic for intercepting calls in generated code.

#7009 (comment)

The biggest drawback is probably spooky action at a distance. I understand that by marking my method interceptable I acquiesce to having my logic replaced, but now the user of my library Foo can reference another library Bar that silently generates interceptors for my method that do whatever they want.

Yeah, we would potentially want something like "Key tokens" listed under "Alternatives" to reduce the risk of this.

#7009 (comment)

The second case is kind of like a horizontal new

I think this is a great point. I would like to try looking at the problem from a different angle, where we consider if some controlled facility could be introduced to make certain extensions better matches than instance methods in some cases. Could this give us some of the benefits of replace/original while still being able to work across projects--that is, where the replacement isn't in the same project as the original.

I don't yet know in which cases this would be or what drawbacks this would have. There could be some fairly spooky problems where the replacement method wouldn't actually get used in places where we want it to.

What I am seeing is that some scenarios like ASP.NET minimal API or query providers want to dispatch based on location essentially because some information about lambda expressions is lost after compilation or requires reflection or complex traversal to access at runtime. But other places want to dispatch based on:

• the value of a string argument, like Regex,
• a System.Type or the 'typeof' a type parameter, like serialization and dependency injection
• nothing in particular! Vectorization for example would probably be simpler to handle with a replace/original approach, since it is using tests which apply to the entire runtime environment to decide whether/how to specialize.

Basically, what if a specialized implementation of a method could be added to a project, which dispatches however the implementer wants, simply by using the flow-control constructs of the language, rather than sticking declarative references to source locations into attributes on the signature.

Consider the following extremely strawman sample, which hopefully gets the basic idea across.

IRouteBuilder builder = ...;
// optionally: "conversion" to an explicit-extension type? where the framework exposes some path for doing this.
SpecializedRouteBuilder builder1 = builder.WithOpportunisticSpecialization();
builder1.MapGet("/products", (request, response) => ...);
builder1.MapGet("/items", (request, response) => ...);
builder1.MapGet("/config", (request, response) => ...);

// WebApp.generated.cs
// make it an optimization problem?
explicit extension SpecializedRouteBuilder for IRouteBuilder
{
    // add an extension which deliberately wins over the one actually present in the type?
    public shadowing IRouteBuilder MapGet(string route, Delegate handler,
        [CallerFilePath] string filePath = null,
        [CallerLineNumber] int lineNumber = -1,
        [CallerColumnNumber] int columnNumber = -1)
    {
        switch (filePath, lineNumber, columnNumber)
        {
            // assume Native AOT can inline and constant-propagate to ensure
            // this dispatch doesn't actually need to happen at runtime in most cases.
            case ("file.cs", 1, 1): Method1(route, handler); return this;
            case ("file.cs", 2, 2): Method2(route, handler); return this;
            case ("file.cs", 3, 3): Method3(route, handler); return this;
            case ("file.cs", 4, 4): Method4(route, handler); return this;
            case ("file.cs", 5, 5): Method5(route, handler); return this;
            case ("file.cs", 6, 6): Method6(route, handler); return this;
            default: base.MapGet(route, handler);
        }
    }
}

// Regex.generated.cs
explicit extension MyRegexFactory for Regex
{
    // add an extension which deliberately wins over the one actually present in the type?
    public static shadowing Regex IsMatch(string pattern)
    {
        switch (pattern)
        {
            case ("a+"): return APlusMatcher();
            case ("ab+"): return ABPlusMatcher();
            default: return base.IsMatch(pattern);
        }
    }
}

[vladd]

@RikkiGibson

I wasn't sure what IPNC means.

I think it’s a common desire to decorate a property in some way and get the boilerplate for implementing INotifyPropertyChange automagically applied. This scenario is not supported by current source generators.

[alrz]

There could be some fairly spooky problems where the replacement method wouldn't actually get used in places where we want it to.

If I understood the extension proposal correctly, an implicit extension always applies? a la #4029

What I am seeing is that some scenarios like ASP.NET minimal API or query providers want to dispatch based on location

I don't think merely the "location" is the defining factor here (just like the IsMatch case), I would expect any c.InterceptableMethod(1) dispatches to the same generated code if the argument is the same.

Of course, this isn't workable for typeof, lambdas or generics (basically anything that doesn't have a const representation).

For generics, it could be a case for specialization:

extension for Serializer {
    // `new` makes sure that this is specialization/shadowing so the signature must match.
    public static new string Serialize</*const*/ MyType>() { .. } 
}

Which can be enabled for constants as well:

extension for Regex {
    public static new bool IsMatch(string str, const string pattern = "abc") { .. }
}

These would result in a separate method and eliminate the need for a switch.

None of that would work for lambdas but I think it should remain an implementation detail, that is, to use some auxiliary APIs to help with generating the switch and getting the "concrete" value (typeof, lambda body, etc) for the generator to use directly.

[HaloFour]

@RikkiGibson

I definitely want us to consider if the use case of the platform specializing calls to platform methods within a project is better served by an post-compilation IL rewriting step. That will require revisiting any reasons we've been reluctant to introduce such a step in the past.

It was mentioned on Discord that this is because IL-rewriting is challenging and the tools aren't up to snuff, and I don't doubt it. Regardless of these use cases I think there's a good opportunity for the .NET teams to come together and provide such tooling, that can emit appropriate IL and debugging symbols. For the most part, writing transformers and aspects In the Java world is a pretty simple experience. It's extremely rare to emit raw byte code, you typically write normal Java classes to describe the advice and use either annotations or a fluent DSL to describe where that advice will be applied. Tooling in IDEs like IntelliJ will show you where the advice will be applied. Transformation can then be performed as a post-compilation step, prior to a native/AOT build. It's pretty slick and easy to use once you grok the concepts of AOP and interception. I would imagine an official solution for .NET, with tooling support, could very easily rival it, too.