dotnet
[API proposal] Attribute for passing caller identity implicitly
Rationale
Number of libraries, including the new minimal ASP.NET host, use Assembly.GetCallingAssembly() for convenience. Assembly.GetCallingAssembly() is slow and unreliable API, and also not compatible with AOT technologies (e.g. https://github.com/dotnet/runtime/issues/53825). We need a fast, reliable and AOT-friendly replacement of Assembly.GetCallingAssembly().
Proposal
namespace System.Runtime.CompilerServices
{
[AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = false)]
public sealed class CallerIdentityAttribute : Attribute
{
public CallerIdentityAttribute();
}
}
This attribute can be applied to parameters of type System.Reflection.Assembly, System.Reflection.Type, System.Reflection.MethodBase. When the C# compiler sees parameter tagged with this attribute, it will provide default value for it, according to the caller context:
-
System.Reflection.Assembly:typeof(containing_type).Assembly -
System.Reflection.Type:typeof(containing_type) -
System.Reflection.MethodBase:MethodBase.GetMethodFromHandle(ldtoken containing_method)
Design meetings
- https://github.com/dotnet/csharplang/blob/main/meetings/2021/LDM-2021-09-22.md#calleridentityattribute
- https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-01-24.md#attribute-for-passing-caller-identity-implicitly
Tagging subscribers to this area: @dotnet/area-system-runtime See info in area-owners.md if you want to be subscribed.
Issue Details
Rationale
Number of libraries, including the new minimal ASP.NET host, use Assembly.GetCallingAssembly() for convenience. Assembly.GetCallingAssembly() is slow and unreliable API, and also not compatible with AOT technologies (e.g. https://github.com/dotnet/runtime/issues/53825). We need a fast, reliable and AOT-friendly replacement of Assembly.GetCallingAssembly().
Proposal
namespace System.Runtime.CompilerServices
{
[AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = false)]
public sealed class CallerIdentityAttribute : Attribute
{
public CallerIdentityAttribute();
}
}
This attribute can be applied to parameters of type System.Reflection.Assembly, System.Reflection.Type, System.Reflection.MethodBase. When the C# compiler sees parameter tagged with this attribute, it will provide default value for it, according to the caller context:
-
System.Reflection.Assembly:typeof(containing_type).Assembly -
System.Reflection.Type:typeof(containing_type) -
System.Reflection.MethodBase:MethodBase.GetMethodFromHandle(ldtoken containing_method)
| Author: | jkotas |
|---|---|
| Assignees: | - |
| Labels: |
|
| Milestone: | - |
So in the case of ASP.NET Core, it would look like this:
public class WebApplication
{
public static WebApplicationBuilder CreateBuilder(string[] args, [CallerIdentityAttribute]Assembly assembly = null);
}
Usage:
var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();
app.MapGet("/", context => context.Response.WriteAsync("Hello World!"));
app.Run();
The compiler would do this (since we're in Main):
var builder = WebApplication.CreateBuilder(args, typeof(Program).Assembly);
var app = builder.Build();
app.MapGet("/", context => context.Response.WriteAsync("Hello World!"));
app.Run();
That's slick. I like it.
We should consider marking Assembly.GetCallingAssembly as obsolete as part of this feature.
Also see: https://github.com/dotnet/csharplang/discussions/87
If those attributes were collapsed into a single attribute that passed a CallerInfo-like struct and the compiler were to pass the runtime method handle in that struct you should be able to derive the assembly from the declaring type of the calling method.
I expect that we will frequently need to add trimming annotations like DynamicallyAccessedMembersAttribute to this argument. CallerInfo-like struct would not work well with these trimming annotations.
Overall this looks pretty reasonable to me.
For the Type and MethodBase values do we want to emit the actual types / methods in which the call occurs or the declared types. Consider as a concrete example the following:
class Program {
static void Main() {
PrintMe();
var x = () => PrintMe();
x();
}
static void PrintMe([CallerIdentityAttribute]Type type = null) => Console.WriteLine(type.FullName);
}
Does this print
Program
Program
Or
Program
<generated type name>
Good question. I think it depends on the usage. Should this work?
class Program {
static void Main() {
PrintMe();
var x = () => PrintMe();
x();
}
static void PrintMe([CallerIdentityAttribute]Type type = null)
{
var m = type.GetMethod(nameof(PrintMe), BindingFlags.Public | BindingFlags.Static); // Does this return null?
}
}
var m = type.GetMethod(nameof(PrintMe), BindingFlags.Public | BindingFlags.Static); // Does this return null?
That is explicitly undefined :smile:.
The compiler does not specify how lambdas are emitted, except in special cases, because we want to be free to change it between versions. Hence the relationship in terms of metadata between PrintMe and the method generated for x is not defined. If the code does work in one version it doesn't imply it will work in the next version.
I know that often the language / framework says an item in metadata is undefined but in reality it never changes hence developers feel safe in taking dependencies on it. Lambda metadata is one area we've changed several times though and broken developers who took dependencies on it.
do we want to emit the actual types / methods in which the call occurs or the declared types
I think it makes more sense to emit the declared types/methods, given that it is not specified how lambdas and other similar constructs are emitted.
To make example in https://github.com/dotnet/csharplang/issues/4984#issuecomment-889724741 work as written, how are we going to retrofit the existing method to use the new pattern?
public class WebApplication
{
// Existing method
public static WebApplicationBuilder CreateBuilder(string[] args);
// New method
public static WebApplicationBuilder CreateBuilder(string[] args, [CallerIdentityAttribute] Assembly assembly = null);
}
I assume that WebApplication.CreateBuilder(args) is going to bind to the existing method. What is the best way to switch the existing code to the overload that uses this feature?
Believe we had the same overload issue with CallerArgumentExpression but not sure exactly what conclusion we came to. @333fred should remember
Believe we had the same overload issue with CallerArgumentExpression but not sure exactly what conclusion we came to
Yeah, it'd be nice if you had:
Debug.Assert(condition);
that we could change the method:
public static void Assert([DoesNotReturnIf(false)] bool condition, string? message)
to instead be:
public static void Assert([DoesNotReturnIf(false)] bool condition, [CallerArgumentExpression("condition")] string? message = null)
such that the compiler would emit:
Debug.Assert(condition, "condition");
Unfortunately, we already have the void Assert(bool condition) overload, so the compiler will bind to the "wrong" one.
So we need to add this overload now (to WebApplication) so we can make this work in the future.
So we need to add this overload now (to WebApplication) so we can make this work in the future.
We already have the existing overload. How does adding the new overload now help (assuming the attribute would have similar semantics to the existing attributes in terms of overload resolution)?
I mean for WebApplication (a new API being added by ASP.NET Core), not Debug.Assert
I mean for WebApplication (a new API being added by ASP.NET Core), not Debug.Assert
Ok, but if you add both:
M(string[] args);
M(string[] args, Assembly? assembly = null);
today, calls to M(args) are going to bind to M(string[] args) today, and in the future when the latter argument is attributed, the compiler will still prefer to bind to M(string[] args).
If what you mean is you would need to add M(string[] args, Assembly? assembly = null) and not add (or remove if it's already added but hasn't shipped) M(string[] args), then yes.
If what you mean is you would need to add M(string[] args, Assembly? assembly = null) and not add (or remove if it's already added but hasn't shipped) M(string[] args), then yes.
This is what I meant, yes
The overload issue is a problem here because we're effectively stuck with the overload we don't want to be used anymore. With Debug.Assert / [CallerInfoExpression] it was more of a convenience problem. We'd really like to use it in Debug.Assert but the product can move forward without it. With [CallerIdentity] it could be important to the success of AOT. Basically if we can't move existing APIs to leverage this feature then it seemingly puts AOT at a bit of risk. That ups the stakes a bit.
The imperfect solution I'd been thinking of in my head was ... yet another feature!!! 😄
Essentially imagine an attribute named [BinaryCompatOnlyAttribute]. When a method is marked with this attribute then the compiler will never include it in a candidate set for purposes of overload resolution. The method exists purely for binary compatibility reasons.
That would allow us to solve this problem rather easily:
public class Debug {
[BinaryCompatOnly]
public static void Assert([DoesNotReturnIf(false)] bool condition) { ... }
public static void Assert([DoesNotReturnIf(false)] bool condition, [CallerArgumentExpression("condition")] string? message = null) { ... }
The burden would be on the API author to ensure they didn't create source breaks when doing this. For the majority use case here though that works out just fine. Given that the extra parameters are optional and we fill in the values existing invocations will just work.
The problem comes in though when we get into method group conversions.
Action<string> action = Debug.Assert;
That would not work because the new overload isn't compatible with Action<string>. That is where this idea starts to fall apart. I thought a bit about saying we only consider [BinaryCompatOnly] for method invocation, not method group conversions but that seems wrong + incomplete.
Action<bool> action = Debug.Assert; is already an error because it's conditional.
Essentially imagine an attribute named [BinaryCompatOnlyAttribute]. When a method is marked with this attribute then the compiler will never include it in a candidate set for purposes of overload resolution. The method exists purely for binary compatibility reasons.
That's a fascinating idea, and I'd want to look at it in conjunction with other ideas that could affect overload resolution. We've had other ideas around attributes that could serve as tiebreakers if two APIs would otherwise be considered ambiguous, and this seems like it might either be a good proving ground for such an attribute, or a good thing to bundle at the same time.
Action
action = Debug.Assert; is already an error because it's conditional.
In my defense, I've only been back from vacation for three hours now. 😄
What if the old overload was only kept in runtime assemblies, but removed from reference assemblies? I think this would mean switching to the new overloads with just recompilation, while maintaining both binary and source compatibility (except for method group conversions, as mentioned above).
It's only a practical solution for large libraries (where it's worth having separate reference assemblies), but that's also where compatibility matters the most.
We've had other ideas around attributes that could serve as tiebreakers if two APIs would otherwise be considered ambiguous
For reference, https://github.com/dotnet/csharplang/discussions/4867 is a recent discussion on that topic.
What if the old overload was only kept in runtime assemblies, but removed from reference assemblies?
That is a viable option but only in a specific subset of scenarios. Mostly when the code is written in C# and it involves method invocation. It becomes unviable in C# method group conversions, F# (likely at least because they have diff overload resolution and method group rules), when the method is an instance method on a non-sealed type, etc ...
The instance method on an unsealed type is the easiest deal breaker. Have to assume that someone derived from you and bound that method to an interface implementation.
It would be still a problem if it was NOT conditional though.
To be fair, at the moment, overloading M(object) with M(object, [AnyCallerInfo] string = null) is just wrong because the compiler is expected to fill in caller info arguments (read "optional args with special semantics").
So the only reason for this to exist is to maintain backward compatibility.
I'd say we should never remove those from the candidate set and prefer the ones with caller info because, as far as the compiler concerns, those are "more specific" overloads since we know how to provide the missing arg(s) with proper values.
I'd say we should never remove those from the candidate set and prefer the ones with caller info because, as far as the compiler concerns, those are "more specific" overloads since we know how to provide the missing arg(s) with proper values.
IMO, the problem with this is that, as the language exists today, they're not more specific. In fact, the language is very specifically worded to say that they are most definitely less specific. I'd worry about unintentional breaking changes if we were to move forward with that, whereas BinaryCompatOnly would be an intentional opt-in.
I'd worry about unintentional breaking changes
This is indeed a breaking change but one of those that corrects the broken code.
This is indeed a breaking change but one of those that corrects the broken code.
I don't think it's fair to label every case where we picked an overload that didn't use caller info args over one that did as broken. It's part of how the feature was explicitly designed. It's possible that customers rely on this behavior (as it's been the designed behavior for 10+ years now).
Whether or not the code is broken / wrong is best decided by the API author. They are the ones best setup to decide if the new overload is the preferred one. That is why I've been thinking of solutions where they could strategically control the "breaks" on existing APIs.