Extensions: ref safety analysis #77967
Conversation
jcouv
added
Area-Compilers
Feature - Extension Everything
dotnet-issue-labeler
bot
added
the
untriaged
|
@AlekseyTs @jjonescz for review. Thanks |
| @@ -2100,6 +2132,57 @@ private SafeContext GetInvocationEscapeWithUpdatedRules( | |||
| return escapeScope; | |||
| } | |||
|
|
|||
| private void ReplaceWithExtensionImplementationIfNeeded(ref MethodInfo methodInfo, ref ImmutableArray<ParameterSymbol> parameters, | |||
There was a problem hiding this comment.
| private void ReplaceWithExtensionImplementationIfNeeded(ref MethodInfo methodInfo, ref ImmutableArray<ParameterSymbol> parameters, | |
| private static void ReplaceWithExtensionImplementationIfNeeded(ref MethodInfo methodInfo, ref ImmutableArray<ParameterSymbol> parameters, | |
| ``` #Pending |
| var extensionParameter = symbol.ContainingType.ExtensionParameter; | ||
| Debug.Assert(extensionParameter is not null); | ||
| Debug.Assert(receiver is not null); | ||
| methodInfo = methodInfo.ReplaceWithExtensionImplementation(out bool wasError); |
There was a problem hiding this comment.
Is it intentional to change the ref MethodInfo methodInfo even if the replacement is an error? #Pending
|
|
||
| if (argRefKindsOpt.IsDefault) | ||
| { | ||
| if (extensionParameter.RefKind == RefKind.Ref) |
There was a problem hiding this comment.
Why do we handle only Ref and not other ref kinds? #Pending
There was a problem hiding this comment.
It was because of the logic in GetInvocationArgumentsForEscape, but I suspect that logic my be wrong for the receiver parameter of classic or new extension methods. I'm still trying to isolate a scenario that would confirm... Let's discuss on Monday
| if (!argsToParamsOpt.IsDefault) | ||
| { | ||
| var argsToParamsBuilder = ArrayBuilder<int>.GetInstance(argsToParamsOpt.Length + 1); | ||
| argsToParamsBuilder.Add(0); |
There was a problem hiding this comment.
Isn't there some extension method like ImmutableArray.Prepend that does all this for us? Or perhaps
argsToParamsOpt = [0, .. argsToParamsOpt];
``` #ByDesignThere was a problem hiding this comment.
We're also trying to shift the values (+1 on line 2179)
Analyze the receiver for an extension as an argument.
Properties are already analyzed as invocations.
Relates to test plan #76130