Fix 'quick info' and 'use explicit type' for implicit lambdas#55290
Fix 'quick info' and 'use explicit type' for implicit lambdas#55290CyrusNajmabadi merged 6 commits intodotnet:mainfrom
Conversation
|
|
||
| public static bool IsAnonymousDelegateType([NotNullWhen(returnValue: true)] this ISymbol? symbol) | ||
| => symbol.IsAnonymousType() && symbol.IsDelegateType(); | ||
| => symbol.IsDelegateType() && (symbol.IsAnonymousType() || !symbol.CanBeReferencedByName); |
There was a problem hiding this comment.
vb and C# are a bit weird here. VB represents anonymous lambdas as actual .IsAnonymousType. C# does not, but instead has CanBeReferencedByName=false.
There was a problem hiding this comment.
Was there an explicit decision here by the compiler team to use CanBeReferencedByName over IsAnonymousType as the api shape? Wish these could be consistent
There was a problem hiding this comment.
@cston any thoughts on why VB/C# differ here wrt anonymous delegates?
There was a problem hiding this comment.
It feels like SynthesizedDelegateSymbol.IsAnonymousType should return true. Please feel free to change that or log an issue.
|
@jinujoseph Need approval on this. This is for some high profile ugliness cases we get with the new anonymous-lambda work that compiler just merged in. |
|
@dotnet/roslyn-compiler for a second review, thanks. |
|
Moving over to main. |
333fred
left a comment
There was a problem hiding this comment.
LGTM, assuming my question around localization is answered with "no, we don't do that for this type."
|
|
||
| public static bool IsAnonymousDelegateType([NotNullWhen(returnValue: true)] this ISymbol? symbol) | ||
| => symbol.IsAnonymousType() && symbol.IsDelegateType(); | ||
| => symbol.IsDelegateType() && (symbol.IsAnonymousType() || !symbol.CanBeReferencedByName); |
| $$var v = (int i) => i.ToString(); | ||
| } | ||
| }", | ||
| MainDescription("delegate TResult System.Func<in T, out TResult>(T arg)"), |
There was a problem hiding this comment.
Shouldn't this just be System.Func<...>? I don't understand the combination of System.Func... with delegate and a method signature. #Closed
There was a problem hiding this comment.
So Func is a delegate, so we show that (just like we say if something is a class/struct). And for delegates we show the signature as that's what's relevant a lot of the time. So this is standard for how we've shown this information for about 15 years :-)
| { | ||
| void M() | ||
| { | ||
| var v = (ref int i) => i.ToString(); |
There was a problem hiding this comment.
Could you include a similar test for a "simple signature" (no ref)? #Closed
| @@ -319,17 +319,13 @@ public static bool ContainsAnonymousType([NotNullWhen(returnValue: true)] this I | |||
|
|
|||
| private static bool ContainsAnonymousType(INamedTypeSymbol type) | |||
There was a problem hiding this comment.
Can we remove this method and overload on line 309?
There was a problem hiding this comment.
i don't really know what you mean. How would we remove it?
There was a problem hiding this comment.
I'm suggesting we delete lines 309 through 332.
There was a problem hiding this comment.
Why would we do that?
There was a problem hiding this comment.
Arg. My bad. When I'd done a FindAllRefs, I'd only found those two methods referencing each other... But in actuality they are used...
Good to go then. Thanks
There was a problem hiding this comment.
basically, i don't understand the motivation here. why would we remove these helpers? and what is your suggestion on what to do about the breakage?
There was a problem hiding this comment.
got it :) ok.
jcouv
left a comment
There was a problem hiding this comment.
Done with review pass (iteration 6)
jcouv
left a comment
There was a problem hiding this comment.
LGTM Thanks (iteration 6) modulo question left about removing method rather than updating it.
No description provided.