Skip to content

Fix Local-Symbol Containing-Symbol in New-Extension-Method-Rewriter for Capture Analysis #78160

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

Merged
merged 15 commits into from
Apr 18, 2025
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ internal class AsyncMethodToStateMachineRewriter : MethodToStateMachineRewriter

private readonly Dictionary<BoundValuePlaceholderBase, BoundExpression> _placeholderMap;

/// <summary>
/// Containing Symbols are not checked after this step - for performance reasons we can allow inaccurate locals
/// </summary>
protected override bool EnforceAccurateContainerForLocals => false;

internal AsyncMethodToStateMachineRewriter(
MethodSymbol method,
int methodOrdinal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace Microsoft.CodeAnalysis.CSharp
internal abstract class BoundTreeToDifferentEnclosingContextRewriter : BoundTreeRewriterWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator
{
// A mapping from every local variable to its replacement local variable. Local variables are replaced when
// their types change due to being inside of a generic method. Otherwise we reuse the original local (even
// their types change due to being inside of a generic method. Otherwise we may reuse the original local (even
// though its containing method is not correct because the code is moved into another method)
private readonly Dictionary<LocalSymbol, LocalSymbol> localMap = new Dictionary<LocalSymbol, LocalSymbol>();

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

protected abstract MethodSymbol CurrentMethod { get; }

protected abstract bool EnforceAccurateContainerForLocals { get; }

public override BoundNode DefaultVisit(BoundNode node)
{
Debug.Fail($"Override the visitor for {node.Kind}");
Expand All @@ -55,7 +57,8 @@ protected virtual bool TryRewriteLocal(LocalSymbol local, [NotNullWhen(true)] ou
}

var newType = VisitType(local.Type);
if (TypeSymbol.Equals(newType, local.Type, TypeCompareKind.ConsiderEverything2))
if (TypeSymbol.Equals(newType, local.Type, TypeCompareKind.ConsiderEverything2) &&
(!EnforceAccurateContainerForLocals || local.ContainingSymbol == CurrentMethod))
{
newLocal = local;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ internal sealed partial class ClosureConversion : MethodToClassRewriter
/// </summary>
private readonly ImmutableHashSet<Symbol> _allCapturedVariables;

/// <summary>
/// Containing Symbols are not checked after this step - for performance reasons we can allow inaccurate locals
/// </summary>
protected override bool EnforceAccurateContainerForLocals => false;

#nullable enable

private ClosureConversion(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ internal sealed class ExtensionMethodBodyRewriter : BoundTreeToDifferentEnclosin

private RewrittenMethodSymbol _rewrittenContainingMethod;

/// <summary>
/// To allow regular capture analysis we do not want to reuse locals with an incorrect containing symbol
/// </summary>
protected override bool EnforceAccurateContainerForLocals => true;

public ExtensionMethodBodyRewriter(MethodSymbol sourceMethod, SourceExtensionImplementationMethodSymbol implementationMethod)
{
Debug.Assert(sourceMethod is not null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ internal IteratorMethodToStateMachineRewriter(
_nextFinalizeState = slotAllocatorOpt?.GetFirstUnusedStateMachineState(increasing: false) ?? StateMachineState.FirstIteratorFinalizeState;
}

/// <summary>
/// Containing Symbols are not checked after this step - for performance reasons we can allow inaccurate locals
/// </summary>
protected override bool EnforceAccurateContainerForLocals => false;

#nullable disable
protected sealed override HotReloadExceptionCode EncMissingStateErrorCode
=> HotReloadExceptionCode.CannotResumeSuspendedIteratorMethod;
Expand Down
Loading