Replies: 20 comments
-
Interesting. I also play with it a little more and found out that Might be related |
Beta Was this translation helpful? Give feedback.
-
Yep, the spec mentions that too, right before the bit above:
|
Beta Was this translation helpful? Give feedback.
-
@yaakov-h If so then I don't think this is a bug. Maybe a bug in compiler warning that it not warn anything about this |
Beta Was this translation helpful? Give feedback.
-
Maybe, except the spec explicitly says that for the purposes of hiding or overriding - which is exactly what we are doing here - " |
Beta Was this translation helpful? Give feedback.
-
@yaakov-h I see then I think it was a bug from the point that |
Beta Was this translation helpful? Give feedback.
-
have you tried this on older c# compilers? I'm wondering for how long it was unnoticed. |
Beta Was this translation helpful? Give feedback.
-
According to a coworker this behaviour has been around for years. |
Beta Was this translation helpful? Give feedback.
-
I just tried this with Mono's C# compiler, and with http://rextester.com which claims to use C# compiler version 4.0.30319.17929. (If I understand correctly, this is a pre-Roslyn version of the C# compiler.) They all produce the same output as shown above. So this behaviour has probably been around for a long time. It appears to me that when it comes to "signatures", the C# spec differs from the underlying platform's notion of signatures. As far as I can tell, CLI signatures (at least the standalone ones) do not distinguish between The C# spec OTOH explicitly states (as mentioned in the initial post) that Perhaps the C# compiler simply takes a shortcut here and implements its overloading / method hiding rules by directly employing the CLI's
Could it be that the spec writers simply forgot to consider cases where such signature "collisions" appear, not in a single type, but in a type hierarchy? |
Beta Was this translation helpful? Give feedback.
-
I first noticed it around the .NET 4.0/4.5 days, I'm fairly sure I also tested it on a machine that only had .NET 3.5 and VisualStudio 2008 installed (I don't recall if csc was distributed with the framework or visual studio back then). My interpretation of ECMA-334 at the time was that having the methods in the same type was prohibited but different types in the same inheritance chain was ok (if you don't mind crazy code). As @stakx pointed out, the ref and out are distinguished by an attribute and attributes are not considered part of the signature by the runtime, though I have seen various tools behave as though it were. I believe this could be resolved with a MethodImpl record, but I think that might be heading down a problematic path and I would be happy if the C# spec were simply 'clarified' to prohibit this :) |
Beta Was this translation helpful? Give feedback.
-
(@brian-reichle - I need to correct myself there: AFAIK, it isn't actually |
Beta Was this translation helpful? Give feedback.
-
@stakx, I think you might be right, but either way it's not using a modopt or modreq so the end result is the same. IIRC, matching (for the purpose of overriding) is based only on the method name, and signature blob, ParamAttributes.Out is definitely not part of the signature blob. hidebysig is ignored by the runtime (Partition II section 15.4.2), I believe it's supposed to provide guidance for tools so they can match language behaviour. |
Beta Was this translation helpful? Give feedback.
-
Just for the record, here is a/the error report on connect from 2014 |
Beta Was this translation helpful? Give feedback.
-
From the readonly-ref spec:
Moving to roslyn repo as a compiler bug. |
Beta Was this translation helpful? Give feedback.
-
Issue moved to dotnet/roslyn #24347 via ZenHub |
Beta Was this translation helpful? Give feedback.
-
Re-opened, as this turns out not be a compiler bug. Copying comment from @VSadov This is a long standing issue with a design choice that virtual calls in C# are emitted via "least derived" instead of the "most derived" method. It is confusing in cases where inheritance hierarchy as seen by CLR is not the same as seen by C#. So when user sees call to With introduction of named parameters, optional parameters, tuples and other features where the information from the most derived statically known method wins over, such behavior is getting more and more confusing. Every time we see a bug like this we consider a change where we emit calls via the most derived override. In most cases the difference would be unobservable, but the confusing cases like this would be "fixed" and that in itself could be considered a breaking change. I think the evidence is mounting that we should make the fix. This should go back to the dotnet/csharplang. CC:@gafter |
Beta Was this translation helpful? Give feedback.
-
Seems that fix for that C#/Roslyn/CLR joint misbehavior is overdue as the issue is not that rare. It would support significantly improved overload resolution in future and increase language and runtime flexibility in implementing other features which could impact overload resolution: see #886. |
Beta Was this translation helpful? Give feedback.
-
@jcouv - probably need to change the title. The binder and overload/override resolution are correct here and respect the spec. The IL that we emit, however, could be more robust. Emitting calls against the most derived target would reduce the chances that something unexpected gets called. |
Beta Was this translation helpful? Give feedback.
-
The fix would not do anything to the binding. It is basically about removing the following line of code: |
Beta Was this translation helpful? Give feedback.
-
I don't believe that switching to the most derived method would fix this specific issue. It might make the call to |
Beta Was this translation helpful? Give feedback.
-
My understanding of the C# spec is that
ref
andout
are treated separately for method overload resolution:Given the following code,
Baz.M(ref object)
should overrideFoo.M(ref object)
, but at runtime it appears to overrideBar(out object)
instead."Go To Definition" agrees with me, or has made the same misunderstanding (see the comment below).
Expected Output:
Actual Output:
Is this:
Thanks.
Beta Was this translation helpful? Give feedback.
All reactions