-
Notifications
You must be signed in to change notification settings - Fork 4k
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
CallerMemberName should work on target typed new() #49547
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
I cannot reproduce using SharpLab. |
I tested on SharpLab using an old branch and could reproduce the issue. This bug might have got fixed at some point of time without actually knowing about it. A test will be needed to prevent future regressions before closing this issue. |
Why CallerMemberName isn't passed through down to a base constructor call? using System;
using System.Runtime.CompilerServices;
namespace ConsoleApp2
{
class Program
{
static void Main()
{
B b = new();
A a = new();
}
}
public class B : A
{
}
public class A
{
public A([CallerMemberName] string? value = null)
{
Console.WriteLine(this.GetType().Name + value);
}
}
} I get
|
As far as I know it is intended that implicit calls do not pass CallerInfo along. See the difference between the following: |
But there's a strange situation, CallerMemberName attribute is defined, method called, but value still empty. WHO initiate this call? huh? Nobody. Really? Take a look at can't see the difference between caller's order in each case. public class B : A
{
}
public class A
{
public A([CallerMemberName] string? value = null)
{}
} CallerMemberName should omit .ctor and pass real external caller name to the parameter value (btw .NetFiddler shows another picture: Fiddle .NET5 - maybe old FW version) |
while we are at it, why VS2019/2022 with .NET5.0 and .NET 6.0 give me this:
while SharpLab run gives:
? |
I get:
For using System;
using System.Runtime.CompilerServices;
namespace ConsoleApp2
{
class Program
{
static void Main()
{
B b = new();
A a = new();
}
}
public class B : A
{
}
public class A
{
public A([CallerMemberName] string? value = null)
{
Console.WriteLine(this.GetType().Name + value);
}
}
} Which makes sense to me. When you have Note: this has nothing to do with target-typed new. You get the same behavior with: using System;
using System.Runtime.CompilerServices;
namespace ConsoleApp2
{
class Program
{
static void Main()
{
B b = new B();
A a = new A();
}
}
public class B : A
{
public B() { }
}
public class A
{
public A([CallerMemberName] string? value = null)
{
Console.WriteLine(this.GetType().Name + value);
}
}
} The sole issue here is that constructor have no 'caller member name' to pass into that attribute. so you get nothing. |
the sole issue is that when the method is get called, sombody has called it. And we always know who it is. |
We definitely do not know who it is :) When B's constructor calls into 'A', it has no idea who called it. Specifically, think about this code: public B() { } This is translated quite literally to there is no location there to place 'Main' as B has no idea who called it (note the lack of parameters entirely). |
The spec currently says this:
This is presumably because there is no well defined 'name' for a constructor. |
but it's already pass ".ctor" in some cases! without any "well defined name". So the whole current logic is dubious.. and I think it should be revised |
You have two pieces of paper, put them together and pierce it with the needle. Who made a hole in a second piece ? |
Maybe it would make sence to add a parameter to
there's not much sence getting ".ctor"'s for users. Users want real caller names |
I have no idea what that means. You seem to be implying that your code is like two pieces of paper, and the callermembername is the needle that was pierced through. But what i'm saying is that it doesn't work like this at all. You have a constructor for B that literally has no parameter by which to "pierce" this information through with. If you want the information to flow through, you must give a parameter and you must pass it along. There's literally no other way for this to work as the callee has no idea who called it (stack traces are meaningless and have no relation to caller names). As such, only the caller can pass this information in. And the only way for the caller to pass the information is for there to be a parameter for them to pass it through with. |
What is the 'real caller name' for a constructor? |
What should it be revised to? There's no way to pass in 'Main' in your case. The intermediary constructor literally has no parameters. So there's no mechanism by which to pass that name along from your 'Main' method through B's constructor all the way to A's constructor. If you want that behavior, then write B like so: public class B : A
{
public B([CallerMemberName] string? value = null) : base(value) { }
}
public class A
{
public A([CallerMemberName] string? value = null)
{
Console.WriteLine(this.GetType().Name + value);
}
} If you do this, the calling B's constructor will pass in |
I mean name of someone who did call constructor |
Again, there is literally no way for this to work, unless you supply an actual parameter to pass along this information. CallerMemberName isn't magic. What it says is "when this call is emitted, pass the literal name of the containing method as a parameter here". If you have no parameter, there is no way to pass the caller's name. In your case, when you instantiated 'A' you called a constructor that had such an annotated parameter. When you instantiated B you called a constructor that did not. So there's no way to "punch through" this information about the caller all the way to A if you define 'B' that way. If you do want this information passed along, then the actual call that you make needs to be annotated with this attribute. See #49547 (comment) for how to do this. |
too much excessive code. And often I do not want to explicitly declare constructors to keep classes clean and clear |
then this is the limitation.
If you do not provide the mechanism by which the caller can pass in their name... then you will end up in a situation where the callee does not know the name of the caller. This is literally the only means by which this can work. You are choosing to not use the tech created for this purpose in the only manner by which it can solve this problem. You are then complaining that it's not working for you.
Users get real caller names by using the attributes properly. If they don't, they get the behavior here. :-/ |
But 'Main' didn't call If you want this caller to be passed along through several layers of calls, then you will have to do that passing yourself. |
I'll show you what result I want from CallerMemberName attribute. Something like this: var realCallerMemberName = new StackTrace().GetFrames()
.Where(m => m.GetMethod()?.IsConstructor == false)
.Select(x => x.GetMethod()?.Name)
.FirstOrDefault(); |
this will not work. Even basic things like async-calls will break that. I mentioned that here: #49547 (comment) You are conflating 'stack' with 'caller'. These are not related. For example, if i have something as simple as: async Task X()
{
// code with awaits
await Y();
}
async Task Y([CallerMemberName] string? caller = null)
{
Console.WriteLine(caller);
} Then this will print out
Note how there's no reference to 'X' at all in that stack. |
It's just an idea that it should go up the calls pipeline and look for someone who is not constructor |
See the above example. It would print out 'MoveNext'. And, frankly, if htat's what you want, then just write a helper utility function that does exactly what you have there. If you're ok with the perf overhead of using StackTrace, any issues that might arise with it not working in some environments (like release mode), and also ok getting random names like this, then def just use this. However, that's not what CallerMemberName is. CallerMemberName is substituting the name of the caller from teh perspective of C#, not from what actually happens at runtime. If you want hte latter, just use things like StackTrace+Reflection+etc. If you want the former, then you'll need to annotate the methods and pass values along if that's the semantics you want. |
I showed you idea of what I want to get for particular synchronous case with constructors. |
We did. It's: And, if you don't want to write that manually, you can create a source generator to do it for you, generating into a file you never see.
No. there is no way with StackTrace to solve the async-case (and may other cases). The reason for that is, as mentioned before: there is no relation between 'caller' and 'stack'. Sometimes they are the same, but it's very easy for them to not be. Consider something basic like an iterator. The caller that created the iterator may be completed unrelated to the stack that is actually iterating it. Similarly with anything related to lambdas and whatnot. Indeed, that's why CallerMemberName exists. Because people want a way to encode information at compile time that is lost at runtime.
Here's the thing: You can't say you want smething, but then both disregard the actual solutions that get you that, and then handwave away that there is no feasible solution to get what you want otherwise. The feature is literally designed hte way it is because the other approaches that you are alluding to are not capable of actually making this work. There is nothin to invent to get you the caller name using the stack when there literally is nothing on hte stack that contains that info. |
By the way, following your logic, we should get empty CallerMemberName for anonymous methods using System;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Linq;
using System.Threading.Tasks;
namespace ConsoleApp2
{
class Program
{
static async Task Main()
{
await Task.Run(()=> {new A();});
var x = ()=> {return new A();};
x();
}
}
public class A
{
public A([CallerMemberName] string value = null)
{
Console.WriteLine(this.GetType().Name + value);
new StackTrace().GetFrames().Take(3).ToList()
.ForEach(x => Console.WriteLine("\t" + x.GetMethod()?.Name));
}
}
} in both cases we get caller = Main. Why? We see anonymous method here! No name, nothing! |
Because that's literally what CallerMemberName does. It substitutes in the name of the containing method at compile time. In other words, it translates the above exactly to: await Task.Run(()=> {new A("Main");});
var x = ()=> {return new A("Main");};
x(); |
You haven't explained how that would work at all. Again, the caller has to supply their name to the callee. The callee cannot say "omit constructors" as that doesn't solve the question of: how does the omitted constructor know the name of what called it? The omitted constructor can only know the name of what called it if it itself has a CallerMemberName parameter on it. In other words, you have three enties:
So, we know 'Y' wants this name. So you start by annotating 'Y' like you've requested: void Y([CallerMemberName(OmitConstructor = true)] string? caller = null) { ... } Ok great... but now what? In Think about it. X's constructor could have been called from arbitrary number of places. Each of those places could have a different name. X has absolutely no idea what it's caller was. So how can it possibly let 'Y' know? So, the only way we can make this work is if the 'X' constructor is passed the name of any of its callers. And how do we do that, by adding a There is literally no other solution that can work here. StackTrace is a complete non-starter as StackTrace literally does not contain these names. So, absent some new proposalyou have that solves this, the only viable path is to use CallerMemberName in the prescribedd fashion (which is how it was designed to be used). |
yes...? As spec'ed CallerMemberName gives you the lexically scoped containing member. Anonymous methods are not members. Methods are. See the spec i linked at: https://github.com/dotnet/csharplang/blob/3c8559f186d4c5df5d1299b0eaa4e139ae130ab6/spec/attributes.md#the-callermembername-attribute |
I never said that. I said the spec lays out hte case where things are implementation defined. An anonymous method though that is within one of the cases the spec specifies is completely defined and should work the way it does. Indeed, that helps reinforce my point. the purpose of CallerMemberName is to give you that lexically enclosing name. Something which absolutely may not exist at runtime and may be entirely unrelated to the invocation stack that is actually executing the code. Lambdas are a perfect case of this. A lambda could be invoked on entirely unrelated call-stacks. but the purpose of CallerMemberName is to give you the name of hte containing member where the lambda body was lexically defined. It's purpose is to be about the lexical scope you were within at creation point, and it serves that purpose well. If you want something different, then CallerMemberName is not the right solution for you. Instead, you can consider things like StackTrace as it seems like you're more interested in what the name is of one of the stackframes that's above you at runtime. |
I'm not a developer of c# and do not know Is it possible to get real caller at compile time or at runtime only (maybe via thread's context) I know two things:
When some method make async method call, we should't breake a chain of paternity. Maybe there should be a special mark for those threads, by which we know which method created this code |
A core problem is that you haven't defined what even is 'a real caller'. From the C# language's perspective, this is working as intended. You haven't explained why your perspective is more appropriate (or even fully what it is).
It is not possible at runtime. It literally is information that may not exist at all.
If you want a change to the langauge:
In other words, it's not sufficient to say: i don't like how thi feature works. Instead, you need to explain how you would actually want it to work and how to make that possible. For example, saying you want it to work in some fashion, but having no way for the compiler or runtime to do that makes things a non-starter as we literally can't provide you with something we can't actually implement.
I don't know what this means. I think the problem is that you yourself don't have a good idea of what a "fully working feature" would be. I genuinely don't know, even have read the entire thread a few times. It sounds like you want something akin to a stacktrace... but you want it to somehow see through asynchrony and potential lambda-indirections. however, you also want this to work without you even having to put attributes on some of your code to let the compiler know what you're doing (#49547 (comment)). I'm skeptical that a solution exists here that would be ale to provide you with the functionality you want, with the adde requirement that you not even need to add attributes to things. if you are able to come up with such a design, please open a discussion over at csharplang and we can assess it. However, absent that, no movement on anything will be made here. As evidenced by this conversation:
So the only thing we've garnered is that you do not like the available solution. But without a suitable alternate, we would not change anything here as we'd generally find the available solution sufficient, and would not accept the reasoning that you "do not want" to use it as sufficient to invent something else which you might also not want to use. |
Real caller is the nearest method who initiate current method call or declared async code
static void Main()
{
B b = new B();
}
public class B : A
{
public B() : base ()
{ }
}
public class A
{
public A([CallerMemberName] string? value = null)
{
Console.WriteLine(this.GetType().Name + value);
}
} I believe compiler able to get this information and do substitutions in any case (sync/async/anonymous calls etc). |
how?
It is not. The compiler would have no idea on how to do this. Consider, for example, that B and A may just be metadata-symbols in a compiled dll somewhere. When compiling Main, there is no way for the compiler to have any clue what will happen here, nor does it have any mechanism to pass this informatio along.
How? it would literally be impossible. The IL and metadata for B and A are already fixed in this case. B takes no parameters and has no way to pass any information about what called it to A. As such there is literally nothing the compiler can do here in You say you "believe" the compiler can do this. But you have to actually demonstrate how it would do that. If it helps, i can supply a prebuilt dll containing B and A;. You can then provide any code necessary in your progam's Main method that proves out how you could pass I think you'll find quickly that this is not possible. -- Again, to repeat, it is not sufficient ot say you believe it can be done. You need to show how it can be done. because, from my perspective, it's not actually possible at all. |
Ok. It's impossible to do. I see. But let's clear something: do you agree that if it was working as I say it would be better and more convenient? (do not pay attention to is it impossible to implement or not) |
As for runtime features, we have
so when sync method is executed, it's name may be added into ExecutionContext's special storage. For async methods added into additionally created storage for each thread. so we would just do something like and it'll look into current thread storage, then ExecutionContext storage if thread storage is empty I still believe it's possible to implement it somehow |
see dotnetfiddle example output is
We have both X and Y in stack (SharpLab throws |
Not really. If a method calls one thing, and that thing calls another, then it is the middle thing that was the caller, not the original method. |
Because it's entirely up to the runtime what may or may not happen here. Sometimes when a task finishes, it may choose to inline execution if the next continuation into that frame. But this is not at all certain to happen. |
there was no "await" inside Y method.. that's why call stack different.. copy paste |
That's the point. Any changes here can affect the results. That's why this can not be used. These frames are an implementation of how the runtime schedules things. |
I don't want frames, I want caller name (excluding constructors) to be stored somewhere in ExecutionContext, and to be able to get this info somehow from sync or async code. Call CurrentThread.CallerMemberName or even CurrentThread.GetCallerMethod(). It'll look for callers in current thread (if async code), -> if not found -> Look for it in ExecutionContext Breaking a chain in a stack trace is a sad thing too.. I hope it will be fixed someday (but stacktraces in exceptions already connect the caller method from sync context as far as i know!) |
I do. These facilities come with costs. I do not want to pay those costs unless needed. The way i indicate it is needed is by opting into these capabilities. That's a good thing. |
@feeleen this is not an issue for Roslyn or the C# language then. You are asking for a runtime change. please open a discussion over at dotnet/runtime. |
Description
CallerMemberNameAttribute is not working correctly with the new target typing feature.
The caller should be the same regardless of target typing. However, the attribute is currently not being respected when target typing new(). The program below demonstrates.
Configuration
.NET 5.0.100 (C# 9)
Win10-1909 x64
Regression?
No. New feature in C# 9.
The text was updated successfully, but these errors were encountered: