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

Proposal: CallerCharacterNumberAttribute #3992

Closed
1 of 4 tasks
YairHalberstadt opened this issue Oct 11, 2020 · 25 comments
Closed
1 of 4 tasks

Proposal: CallerCharacterNumberAttribute #3992

YairHalberstadt opened this issue Oct 11, 2020 · 25 comments

Comments

@YairHalberstadt
Copy link
Contributor

YairHalberstadt commented Oct 11, 2020

CallerCharacterNumberAttribute

  • Proposed
  • Prototype: Not Started
  • Implementation: Not Started
  • Specification: Not Started

Summary

Add a CallerCharacterNumberAttribute which when applied on an optional parameter, the compiler replaces with the caller's character (column) number, similar to https://docs.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.callerlinenumberattribute?view=netcore-3.1.

The character number, is "how many characters along is this in the string representing this line".

Motivation

Consider a source generator that generates a method which acts differently depending on where it's called form. To do so it switches on CallerLineNumber, and CallerFilePath.

For example, a source generator might provide a method to print the expression passed into it as an argument (see it on sharplab):

using System;
using System.Runtime.CompilerServices;

Console.WriteLine(Helpers.PrintExpression(1 + 3 + 7));
Console.WriteLine(Helpers.PrintExpression(new object()));
Console.WriteLine(Helpers.PrintExpression(5 == 7));

// This code is all generated by a source generator
public static partial class Helpers 
{
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static string PrintExpression(object expression, [CallerFilePath] string filePath = default, [CallerLineNumber] int lineNumber = default)
    {
        return (lineNumber, filePath) switch {
                (4, "Main.cs") =>  "1 + 3 + 7" ,
                (5, "Main.cs")  => "new object()",
                (6, "Main.cs") => "5 == 7",
                _ => ""
        };    
    }
}

This approach is actually optimized quite nicely right now by the jit (assuming the method is less than 64kb of IL, but that limit may be lifted by .NET 6, and can be alleviated by creating a tree of methods if necessary).

However it won't work at the moment if PrintExpression is called twice on the same line. To differentiate that we'll need access to the caller's character number.

Detailed design

Add an attribute System.Runtime.CompilerServices.CallerCharacterNumberAttribute:

namespace System.Runtime.CompilerServices
{
    [System.AttributeUsage(System.AttributeTargets.Parameter, Inherited=false)]
    public sealed class CallerCharacterNumberAttribute : Attribute {}
}

Everything is as for the other Caller attributes: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/attributes/caller-information, except it provides character number.

The above example could now be written as:

using System;
using System.Runtime.CompilerServices;

Console.WriteLine(Helpers.PrintExpression(1 + 3 + 7));
Console.WriteLine(Helpers.PrintExpression(new object()));
Console.WriteLine(Helpers.PrintExpression(5 == 7));

// This code is all generated by a source generator
public static partial class Helpers 
{
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static string PrintExpression(object expression, [CallerFilePath] string filePath = default, [CallerLineNumber] int lineNumber = default, [CallerCharacterNumber] int characterNumber = default)
    {
        return (lineNumber, characterNumber, filePath) switch {
                (4, 42, "Main.cs") =>  "1 + 3 + 7" ,
                (5, 42, "Main.cs")  => "new object()",
                (6, 42, "Main.cs") => "5 == 7",
                _ => ""
        };    
    }
}

Drawbacks

The main question is whether this is a pattern we want to encourage in the first place. Using the trio of CallerFilePath, CallerLineNumber, and CallerCharacterNumber to distinguish the location something is called from and run something different in each case, is a hacky workaround to get around the no source code rewriting limitation of source generators. It's not clear whether it's any better than source rewriting, and relies heavily on the JIT to do a good job to be efficient.

Alternatives

Do nothing

Unresolved questions

Seeing is this is pretty much identical to CallerLineNumber, I can't imagine there are any unresolved design questions.

Relevant issues/discussions

See #3987 which could be solved using this technique.

Design meetings

https://github.com/dotnet/csharplang/blob/main/meetings/2023/LDM-2023-10-09.md#callercharacternumberattribute
https://github.com/dotnet/csharplang/blob/main/meetings/2024/LDM-2024-09-06.md#callercharacternumberattribute

@CyrusNajmabadi
Copy link
Member

I would call this CallerCharacterNumber. One terminology issue is that 'Column' is generally an IDE concept (and thus affected by things like 'how many spaces are in a tab?', whereas 'char(acter)' just means 'how many characters along is this in the string representing this line.

@YairHalberstadt
Copy link
Contributor Author

Roslyn also uses the terminology Character, so that will be best to avoid confusion, since you'll be getting this info from roslyn in the source generator. I'll update the proposal with this change.

@YairHalberstadt YairHalberstadt changed the title Proposal: CallerColumnNumberAttribute Proposal: CallerCharacterNumberAttribute Oct 11, 2020
@333fred
Copy link
Member

333fred commented Oct 11, 2020

For example, a source generator might provide a method to print the expression passed into it as an argument (see it on sharplab)

A source generator might do this, sure, but I don't really buy that as a use case: C# 10 will hopefully be adding support for CallerArgumentExpression, which would completely handle this case. What real-world use case would this address that wouldn't be handled by existing features?

@YairHalberstadt
Copy link
Contributor Author

@333fred

Another example is given in the linked issue: #3987

This gives more flexibility than CallerArgumentExpression alone. For example, this project https://github.com/SingleAccretion/Accretion.Diagnostics allows you to use this as an extension method: expression.Log() which would print: expression = Value. I would personally make it return the value as well, allowing you to easily log intermediate values as you go.

More generally, this is useful if a source generator wants to customize behavior based on the environment it's called in. I can't think of a really good motivating example not related to logging right now, but I'm pretty sure there are some. Will update when I think of any :-).

@YairHalberstadt
Copy link
Contributor Author

I see CallerArgumentExpression will work for extension method arguments. But the point remains the same - this allows you to e.g. customize the formatting, or include only certain parts of the argument, or include the entire line, or in general customize the output based on the context.

@alrz
Copy link
Contributor

alrz commented Oct 11, 2020

Relates to #87

@YairHalberstadt
Copy link
Contributor Author

@333fred Another example - consider a function which takes a delegate as a parameter, and caches the result of the delegate, so it's only evaluated once, and stores it in a static field. It then switches on the calling location, to find the correct field to return.

e.g.

using System;
using System.Runtime.CompilerServices;

Console.WriteLine(Helpers.Cache(() => 1 + 2 + 3));
Console.WriteLine(Helpers.Cache(() => "Hello" + "World"));
Console.WriteLine(Helpers.Cache(() => new object()));

public static partial class Helpers
{
    private static int? field1;
    private static string field2;
    private static object field3;
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static T Cache<T>(Func<T> func, [CallerFilePath] string filePath = default, [CallerLineNumber] int lineNumber = default)
    {
        return (lineNumber, filePath) switch {
                (4, "_") =>  (T)(object)(field1 ??= (int?)(object)func()),
                (5, "_")  => (T)(object)(field2 ??= (string)(object)func()),
                (6, "_") => (T)(field3 ??= func()),
                _ => default,
        };    
    }
}

I'm sure that more creative people could come up with many more exciting examples.

@333fred
Copy link
Member

333fred commented Oct 11, 2020

That certainly is more interesting, but I don't believe it's very interesting. Those delegates don't depend on state, so you could just use reference equality in the cache (which would likely be faster). Examples that do depend on state couldn't be cached no matter the caching strategy, so that also doesn't matter.

@YairHalberstadt
Copy link
Contributor Author

Those delegates don't depend on state, so you could just use reference equality in the cache.

That's relying on an implementation detail, and is easy to accidentally break - for example by converting a method to a delegate directly, or capturing a local variable which is going to be the same on all invocations.

Still not the most compelling of examples I admit :-)

I think what I'm trying to do here is to create a library of techniques which can be used for source generators, and I'm exploring where and when this technique might be useful. I think the example above does show that it has potential for more than just logging, although I don't think I've come up with a killer use case yet. I am pretty sure though there is somebody out there who is doing something with source generators for which this is the perfect solution. I just need to get them to comment here :-)

@foxesknow
Copy link

Why not just have a [CallerLineDetails] attribute that gives you the row/column as a tuple (int Row, int Column) ?

string PrintExpression(object expression, [CallerFilePath] string filePath = default, [CallerLineDetails] position = default)

It reduces the number of parameters which makes the Intellisense presentation look less cluttered.

@YairHalberstadt
Copy link
Contributor Author

Why not just have a [CallerLineDetails] attribute that gives you the row/column as a tuple (int Row, int Column) ?

I don't expect people to be using this manually at all, so I don't think it's worth optimising for style. This is the simplest to design and implement.

@YairHalberstadt
Copy link
Contributor Author

YairHalberstadt commented Mar 3, 2021

@333fred here's an example I would like to write a generator for, and could benefit from CallerCharacterNumber.

I want to turn a static lambda into a function pointer using source generators. This requires checking what the passed in lambda is by looking at CallerFileNumber, CallerLineNumber and CallerCharacterNumber.

E.g.

using System;
using System.Runtime.CompilerServices;

Console.WriteLine(Eval(Helpers.FP((a, b) => (a + b)), 1, 2));
Console.WriteLine(Eval(Helpers.FP((a, b) => (a * b)), 1, 2));
Console.WriteLine(Eval(Helpers.FP((a, b) => (a - b)), 1, 2));

static int Eval(delegate*<int, int, int> del, int a, int b) => del(a, b);

internal static partial class Helpers
{
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static unsafe delegate*<int, int, int> FP(Func<T, T, T> func, [CallerFilePath] string filePath = default, [CallerLineNumber] int lineNumber = default)
    {
        return (lineNumber, filePath) switch {
                (4, "_") =>  &M1,
                (5, "_")  => &M2,
                (6, "_") => &M3,
                _ => default,
        };    
    }

    static int M1(int a, int b) => a + b;
    static int M2(int a, int b) => a * b;
    static int M3(int a, int b) => a - b;
}

@alrz
Copy link
Contributor

alrz commented Mar 3, 2021

That would be a fairly complex generator to begin with, provided that it doesn't cause some safety issue.

Doesn't it make sense to actually allow lambda assignment to function pointers? (#3476)

(I am not against this feature at all, I just think that particular case should be supported by the language)

@YairHalberstadt
Copy link
Contributor Author

@alrz agreed. However one advantage of source generators is to allow you to fill in where the language is missing features. I want to promote techniques using source generators that allow you to do that.

@bernd5
Copy link
Contributor

bernd5 commented Mar 3, 2021

@YairHalberstadt
How do you do that? AFAIK you can only add code - but not change or remove user written code with source generators. Is that not true anymore?

@YairHalberstadt
Copy link
Contributor Author

@bernd5

Indeed. This proposal shows one way of working around that limitation using a technique which works in some cases.

@333fred
Copy link
Member

333fred commented Mar 3, 2021

Doesn't it make sense to actually allow lambda assignment to function pointers? (#3476)

Indeed, @YairHalberstadt, that's my reaction as well.

However one advantage of source generators is to allow you to fill in where the language is missing features. I want to promote techniques using source generators that allow you to do that.

This is not a goal for me. Source generators aren't about enabling syntax for new languages features, it's about removing boilerplate code. Everything I've seen in this proposal so far is about the former, not the latter, which is why I'm still unenthusastic about it.

@RikkiGibson
Copy link
Contributor

Marking myself as a champion here because it sits in the design space for interceptors, and it's possible this will be an element of the solution we choose.

@333fred 333fred added this to the Working Set milestone Oct 9, 2023
@333fred
Copy link
Member

333fred commented Oct 9, 2023

This is triaged into the working set to accompany #7009.

@333fred 333fred modified the milestones: Working Set, Likely Never Sep 7, 2024
@333fred
Copy link
Member

333fred commented Sep 7, 2024

LDM looked at this here https://github.com/dotnet/csharplang/blob/main/meetings/2024/LDM-2024-09-06.md#callercharacternumberattribute. We are not pursuing this approach to interceptors, so we are closing this out.

@333fred 333fred closed this as not planned Won't fix, can't repro, duplicate, stale Sep 7, 2024
@KennethHoff
Copy link

While I understand the interceptor usecase ( or rather, the lack thereof), this issue feels like it could stand on its own.

@julealgon
Copy link

Wouldn't this feature be useful to allow for identifying which value was null in a complex expression when a NullReferenceException is thrown?

I've went through this problem recently where a very large LINQ Select call with an object initializer caused a null ref due to one specific null access that was hard to debug as there were multiple candidates for being null in the same statement.

@CyrusNajmabadi
Copy link
Member

Could you expand on that @julealgon ?

Also, this likely would not help linq (since those signatures would not be updated).

@julealgon
Copy link

Could you expand on that @julealgon ?

@CyrusNajmabadi Sorry, looking back at that example, it wasn't really a LINQ select, just a normal call with many different arguments:

Here is a slightly sanitized version of it. Yes... pretty bad code... but the point was that identifying exactly which argument computation was causing the null reference here was hard as that information is just not present anywhere in the exception (it points to the call line only):

var response = await service.InsertAsync(submissionRevisionId: Guid.NewGuid(),
  submissionId: submissionRevison.SubmissionId, companyId: submissionRevison.CompanyId,
  odometer: submissionRevison.Odometer,
  vehicleTypeId: (byte?)submissionRevison.VehicleType,
  vin: submissionRevison.Vin, tradeInVehicleId: submissionRevison.TradeInVehicleId,
  createdBy: submissionRevison.CreatedBy,
  createdOn: submissionRevison.CreatedOn,
  otherConsiderations: submissionRevison.OtherConsiderations,
  exteriorColor: (byte?)submissionRevison.VehicleMaterialAndColors.ExteriorColor,
  interiorColor: (byte?)submissionRevison.VehicleMaterialAndColors?.InteriorColor,
  interiorMaterial: (byte?)submissionRevison.VehicleMaterialAndColors?.InteriorMaterial,
  exteriorColorDetails: submissionRevison.VehicleMaterialAndColors?.ExteriorColorDetails,
  interiorColorDetails: submissionRevison.VehicleMaterialAndColors?.InteriorColorDetails,
  interiorMaterialDetails: submissionRevison.VehicleMaterialAndColors?.InteriorMaterialDetails,
  hasAfterMarketModifications: submissionRevison.VehicleConditions?.HasAfterMarketModifications,
  afterMarketModificationsConditionDetails: submissionRevison.VehicleConditions?.AfterMarketModificationsConditionDetails,
  carBeenInAccident: (byte?)submissionRevison.VehicleConditions?.CarBeenInAccident,
  carBeenInAccidentDetails: submissionRevison.VehicleConditions?.CarBeenInAccidentDetails,
  hasCosmeticIssues: submissionRevison.VehicleConditions?.HasCosmeticIssues,
  cosmeticConditionDetails: submissionRevison.VehicleConditions?.CosmeticConditionDetails,
  hasExteriorDamage: submissionRevison.VehicleConditions?.HasExteriorDamage,
  exteriorDamageConditionDetails: submissionRevison.VehicleConditions?.ExteriorDamageConditionDetails,
  hasMechanicalIssues: submissionRevison.VehicleConditions?.HasMechanicalIssues,
  mechanicalIssueConditionDetails: submissionRevison.VehicleConditions?.MechanicalIssueConditionDetails,
  hasPreviousBodyOrPaintWorkInThePanels: submissionRevison.VehicleConditions?.HasPreviousBodyOrPaintWorkInThePanels,
  previousBodyOrPaintWorkInThePanelsDetails: submissionRevison.VehicleConditions?.PreviousBodyOrPaintWorkInThePanelsDetails,
  hasTheOdometerBeenBrokenReplacedOrModified: submissionRevison.VehicleConditions?.HasTheOdometerBeenBrokenReplacedOrModified,
  odometerConditionDetails: submissionRevison.VehicleConditions?.OdometerConditionDetails,
  hasOnboardSystemIssues: submissionRevison.VehicleConditions?.HasOnboardSystemIssues,
  onboardSystemConditionDetails: submissionRevison.VehicleConditions?.OnboardSystemConditionDetails,
  doesTiresNeedToBeReplaced: submissionRevison.VehicleConditions?.DoesTiresNeedToBeReplaced,
  tiresConditionDetails: submissionRevison.VehicleConditions?.TiresConditionDetails,
  conditionType: (byte?)submissionRevison.VehicleConditions?.ConditionType,
  doesVehicleHistoryReportHaveIssuesListed: submissionRevison.VehicleConditions?.DoesVehicleHistoryReportHaveIssuesListed,
  vehicleHistoryReportDetails: submissionRevison.VehicleConditions?.VehicleHistoryReportDetails,
  hasWarningLightsShownOnTheDashboard: submissionRevison.VehicleConditions?.HasWarningLightsShownOnTheDashboard,
  warningLightsConditionDetails: submissionRevison.VehicleConditions?.WarningLightsConditionDetails,
  wheelsType: (byte?)submissionRevison.VehicleConditions?.WheelsType,
  wheelsCondition: (byte?)submissionRevison.VehicleConditions?.WheelsCondition,
  wheelsConditionDetails: submissionRevison.VehicleConditions?.WheelsConditionDetails
);

When a coworker sent this asking for guidance I could identify that the issue was likely on the exteriorColor: argument since that was lacking the defensive check present in other arguments assigned from the same source.

-  exteriorColor: (byte?)submissionRevison.VehicleMaterialAndColors.ExteriorColor,
+  exteriorColor: (byte?)submissionRevison.VehicleMaterialAndColors?.ExteriorColor,

The exception message that is generated for cases such as this lacks enough detail to be able to pinpoint the line/column/argument position, as it goes something like this:

System.NullReferenceException: Object reference not set to an instance of an object.
at async Task Repository.AddSubmissionRevisionAsync(SubmissionRevision submissionRevison) in Repository.cs:line 494...

With a bunch of other frames reaching that specific call but only mentioning the initial call line number.

To be clear, this is a NET472 application. The reason I mention this is that I know there have been changes to stack trace information in later releases of the frammework.

@CyrusNajmabadi
Copy link
Member

Oh my

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants