Skip to content

Commit d329245

Browse files
authored
Fix Local-Symbol Containing-Symbol in New-Extension-Method-Rewriter for Capture Analysis (#78160)
* fixes #78135 * fixes #78135 * comment adjusted * test fixed * comment added * reuse if container does not change * AllowReuseOfLocalsWithIncorrectContainingSymbol => EnforceAccurateContainerForLocals * use CompileAndVerify, acutally use extensions * test added * use VerifyTypeIL * test added for #78042 * avoid Foo * simplify if statement * simplify test - use simple local function without async * EnforceAccurateContainerForLocals should be abstract
1 parent 72266a2 commit d329245

File tree

6 files changed

+511
-2
lines changed

6 files changed

+511
-2
lines changed

src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ internal class AsyncMethodToStateMachineRewriter : MethodToStateMachineRewriter
6363

6464
private readonly Dictionary<BoundValuePlaceholderBase, BoundExpression> _placeholderMap;
6565

66+
/// <summary>
67+
/// Containing Symbols are not checked after this step - for performance reasons we can allow inaccurate locals
68+
/// </summary>
69+
protected override bool EnforceAccurateContainerForLocals => false;
70+
6671
internal AsyncMethodToStateMachineRewriter(
6772
MethodSymbol method,
6873
int methodOrdinal,

src/Compilers/CSharp/Portable/Lowering/BoundTreeToDifferentEnclosingContextRewriter.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace Microsoft.CodeAnalysis.CSharp
1919
internal abstract class BoundTreeToDifferentEnclosingContextRewriter : BoundTreeRewriterWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator
2020
{
2121
// A mapping from every local variable to its replacement local variable. Local variables are replaced when
22-
// their types change due to being inside of a generic method. Otherwise we reuse the original local (even
22+
// their types change due to being inside of a generic method. Otherwise we may reuse the original local (even
2323
// though its containing method is not correct because the code is moved into another method)
2424
private readonly Dictionary<LocalSymbol, LocalSymbol> localMap = new Dictionary<LocalSymbol, LocalSymbol>();
2525

@@ -30,6 +30,8 @@ internal abstract class BoundTreeToDifferentEnclosingContextRewriter : BoundTree
3030

3131
protected abstract MethodSymbol CurrentMethod { get; }
3232

33+
protected abstract bool EnforceAccurateContainerForLocals { get; }
34+
3335
public override BoundNode DefaultVisit(BoundNode node)
3436
{
3537
Debug.Fail($"Override the visitor for {node.Kind}");
@@ -55,7 +57,8 @@ protected virtual bool TryRewriteLocal(LocalSymbol local, [NotNullWhen(true)] ou
5557
}
5658

5759
var newType = VisitType(local.Type);
58-
if (TypeSymbol.Equals(newType, local.Type, TypeCompareKind.ConsiderEverything2))
60+
if (TypeSymbol.Equals(newType, local.Type, TypeCompareKind.ConsiderEverything2) &&
61+
(!EnforceAccurateContainerForLocals || local.ContainingSymbol == CurrentMethod))
5962
{
6063
newLocal = local;
6164
}

src/Compilers/CSharp/Portable/Lowering/ClosureConversion/ClosureConversion.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,11 @@ internal sealed partial class ClosureConversion : MethodToClassRewriter
151151
/// </summary>
152152
private readonly ImmutableHashSet<Symbol> _allCapturedVariables;
153153

154+
/// <summary>
155+
/// Containing Symbols are not checked after this step - for performance reasons we can allow inaccurate locals
156+
/// </summary>
157+
protected override bool EnforceAccurateContainerForLocals => false;
158+
154159
#nullable enable
155160

156161
private ClosureConversion(

src/Compilers/CSharp/Portable/Lowering/ExtensionMethodBodyRewriter.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ internal sealed class ExtensionMethodBodyRewriter : BoundTreeToDifferentEnclosin
2727

2828
private RewrittenMethodSymbol _rewrittenContainingMethod;
2929

30+
/// <summary>
31+
/// To allow regular capture analysis we do not want to reuse locals with an incorrect containing symbol
32+
/// </summary>
33+
protected override bool EnforceAccurateContainerForLocals => true;
34+
3035
public ExtensionMethodBodyRewriter(MethodSymbol sourceMethod, SourceExtensionImplementationMethodSymbol implementationMethod)
3136
{
3237
Debug.Assert(sourceMethod is not null);

src/Compilers/CSharp/Portable/Lowering/IteratorRewriter/IteratorMethodToStateMachineRewriter.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ internal IteratorMethodToStateMachineRewriter(
7474
_nextFinalizeState = slotAllocatorOpt?.GetFirstUnusedStateMachineState(increasing: false) ?? StateMachineState.FirstIteratorFinalizeState;
7575
}
7676

77+
/// <summary>
78+
/// Containing Symbols are not checked after this step - for performance reasons we can allow inaccurate locals
79+
/// </summary>
80+
protected override bool EnforceAccurateContainerForLocals => false;
81+
7782
#nullable disable
7883
protected sealed override HotReloadExceptionCode EncMissingStateErrorCode
7984
=> HotReloadExceptionCode.CannotResumeSuspendedIteratorMethod;

0 commit comments

Comments
 (0)