Skip to content
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.

Sign up for GitHub

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

Jump to bottom

EnC: Defer lambda rude edit reporting to runtime #70418

Merged
merged 5 commits into from
Dec 7, 2023
Merged

EnC: Defer lambda rude edit reporting to runtime #70418

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e749b2e
Report runtime rude edits for lambdas and local functions
tmat Oct 10, 2023
f63dcf8
Fix nullability
tmat Nov 30, 2023
8d39d31
Unused member
tmat Nov 30, 2023
8de9dac
Feedback
tmat Dec 6, 2023
035a1eb
Update src/Compilers/VisualBasic/Portable/Compilation/MethodCompiler.vb
tmat Dec 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 31 additions & 14 deletions src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,9 @@ private static MethodSymbol GetEntryPoint(CSharpCompilation compilation, PEModul
}

VariableSlotAllocator lazyVariableSlotAllocator = null;
var lambdaDebugInfoBuilder = ArrayBuilder<LambdaDebugInfo>.GetInstance();
var closureDebugInfoBuilder = ArrayBuilder<ClosureDebugInfo>.GetInstance();
var lambdaDebugInfoBuilder = ArrayBuilder<EncLambdaInfo>.GetInstance();
var lambdaRuntimeRudeEditsBuilder = ArrayBuilder<LambdaRuntimeRudeEditInfo>.GetInstance();
var closureDebugInfoBuilder = ArrayBuilder<EncClosureInfo>.GetInstance();
var stateMachineStateDebugInfoBuilder = ArrayBuilder<StateMachineStateDebugInfo>.GetInstance();
StateMachineTypeSymbol stateMachineTypeOpt = null;
const int methodOrdinal = -1;
Expand All @@ -272,6 +273,7 @@ private static MethodSymbol GetEntryPoint(CSharpCompilation compilation, PEModul
diagnostics,
ref lazyVariableSlotAllocator,
lambdaDebugInfoBuilder,
lambdaRuntimeRudeEditsBuilder,
closureDebugInfoBuilder,
stateMachineStateDebugInfoBuilder,
out stateMachineTypeOpt);
Expand All @@ -280,10 +282,12 @@ private static MethodSymbol GetEntryPoint(CSharpCompilation compilation, PEModul
Debug.Assert(stateMachineTypeOpt is null);
Debug.Assert(codeCoverageSpans.IsEmpty);
Debug.Assert(lambdaDebugInfoBuilder.IsEmpty());
Debug.Assert(lambdaRuntimeRudeEditsBuilder.IsEmpty());
Debug.Assert(closureDebugInfoBuilder.IsEmpty());
Debug.Assert(stateMachineStateDebugInfoBuilder.IsEmpty());

lambdaDebugInfoBuilder.Free();
lambdaRuntimeRudeEditsBuilder.Free();
closureDebugInfoBuilder.Free();
stateMachineStateDebugInfoBuilder.Free();

Expand All @@ -294,8 +298,9 @@ private static MethodSymbol GetEntryPoint(CSharpCompilation compilation, PEModul
synthesizedEntryPoint,
methodOrdinal,
loweredBody,
ImmutableArray<LambdaDebugInfo>.Empty,
ImmutableArray<ClosureDebugInfo>.Empty,
ImmutableArray<EncLambdaInfo>.Empty,
ImmutableArray<LambdaRuntimeRudeEditInfo>.Empty,
ImmutableArray<EncClosureInfo>.Empty,
ImmutableArray<StateMachineStateDebugInfo>.Empty,
stateMachineTypeOpt: null,
variableSlotAllocatorOpt: null,
Expand Down Expand Up @@ -750,8 +755,9 @@ private void CompileSynthesizedMethods(TypeCompilationState compilationState)
method,
methodOrdinal,
loweredBody,
ImmutableArray<LambdaDebugInfo>.Empty,
ImmutableArray<ClosureDebugInfo>.Empty,
ImmutableArray<EncLambdaInfo>.Empty,
ImmutableArray<LambdaRuntimeRudeEditInfo>.Empty,
ImmutableArray<EncClosureInfo>.Empty,
stateMachineStateDebugInfoBuilder.ToImmutable(),
stateMachine,
variableSlotAllocatorOpt,
Expand Down Expand Up @@ -1138,8 +1144,9 @@ forSemanticModel.Syntax is { } semanticModelSyntax &&
bool hasBody = flowAnalyzedBody != null;
VariableSlotAllocator lazyVariableSlotAllocator = null;
StateMachineTypeSymbol stateMachineTypeOpt = null;
var lambdaDebugInfoBuilder = ArrayBuilder<LambdaDebugInfo>.GetInstance();
var closureDebugInfoBuilder = ArrayBuilder<ClosureDebugInfo>.GetInstance();
var lambdaDebugInfoBuilder = ArrayBuilder<EncLambdaInfo>.GetInstance();
var lambdaRuntimeRudeEditsBuilder = ArrayBuilder<LambdaRuntimeRudeEditInfo>.GetInstance();
tmat marked this conversation as resolved.
Show resolved Hide resolved
var closureDebugInfoBuilder = ArrayBuilder<EncClosureInfo>.GetInstance();
var stateMachineStateDebugInfoBuilder = ArrayBuilder<StateMachineStateDebugInfo>.GetInstance();
BoundStatement loweredBodyOpt = null;

Expand All @@ -1159,6 +1166,7 @@ forSemanticModel.Syntax is { } semanticModelSyntax &&
diagsForCurrentMethod,
ref lazyVariableSlotAllocator,
lambdaDebugInfoBuilder,
lambdaRuntimeRudeEditsBuilder,
closureDebugInfoBuilder,
stateMachineStateDebugInfoBuilder,
out stateMachineTypeOpt);
Expand Down Expand Up @@ -1234,6 +1242,7 @@ forSemanticModel.Syntax is { } semanticModelSyntax &&
diagsForCurrentMethod,
ref lazyVariableSlotAllocator,
lambdaDebugInfoBuilder,
lambdaRuntimeRudeEditsBuilder,
closureDebugInfoBuilder,
stateMachineStateDebugInfoBuilder,
out StateMachineTypeSymbol initializerStateMachineTypeOpt);
Expand Down Expand Up @@ -1285,16 +1294,19 @@ forSemanticModel.Syntax is { } semanticModelSyntax &&
return;
}
}
if (_emitMethodBodies && (!(methodSymbol is SynthesizedStaticConstructor cctor) || cctor.ShouldEmit(processedInitializers.BoundInitializers)))
if (_emitMethodBodies && (methodSymbol is not SynthesizedStaticConstructor cctor || cctor.ShouldEmit(processedInitializers.BoundInitializers)))
{
var boundBody = BoundStatementList.Synthesized(syntax, boundStatements);

lambdaRuntimeRudeEditsBuilder.Sort(static (x, y) => x.LambdaId.CompareTo(y.LambdaId));

var emittedBody = GenerateMethodBody(
_moduleBeingBuiltOpt,
methodSymbol,
methodOrdinal,
boundBody,
lambdaDebugInfoBuilder.ToImmutable(),
lambdaRuntimeRudeEditsBuilder.ToImmutable(),
closureDebugInfoBuilder.ToImmutable(),
stateMachineStateDebugInfoBuilder.ToImmutable(),
stateMachineTypeOpt,
Expand All @@ -1315,6 +1327,7 @@ forSemanticModel.Syntax is { } semanticModelSyntax &&
finally
{
lambdaDebugInfoBuilder.Free();
lambdaRuntimeRudeEditsBuilder.Free();
closureDebugInfoBuilder.Free();
stateMachineStateDebugInfoBuilder.Free();
}
Expand All @@ -1338,8 +1351,9 @@ internal static BoundStatement LowerBodyOrInitializer(
out ImmutableArray<SourceSpan> codeCoverageSpans,
BindingDiagnosticBag diagnostics,
ref VariableSlotAllocator lazyVariableSlotAllocator,
ArrayBuilder<LambdaDebugInfo> lambdaDebugInfoBuilder,
ArrayBuilder<ClosureDebugInfo> closureDebugInfoBuilder,
ArrayBuilder<EncLambdaInfo> lambdaDebugInfoBuilder,
ArrayBuilder<LambdaRuntimeRudeEditInfo> lambdaRuntimeRudeEditsBuilder,
ArrayBuilder<EncClosureInfo> closureDebugInfoBuilder,
ArrayBuilder<StateMachineStateDebugInfo> stateMachineStateDebugInfoBuilder,
out StateMachineTypeSymbol stateMachineTypeOpt)
{
Expand Down Expand Up @@ -1407,8 +1421,9 @@ internal static BoundStatement LowerBodyOrInitializer(
method.ThisParameter,
method,
methodOrdinal,
null,
substitutedSourceMethod: null,
lambdaDebugInfoBuilder,
lambdaRuntimeRudeEditsBuilder,
closureDebugInfoBuilder,
lazyVariableSlotAllocator,
compilationState,
Expand Down Expand Up @@ -1453,8 +1468,9 @@ private static MethodBody GenerateMethodBody(
MethodSymbol method,
int methodOrdinal,
BoundStatement block,
ImmutableArray<LambdaDebugInfo> lambdaDebugInfo,
ImmutableArray<ClosureDebugInfo> closureDebugInfo,
ImmutableArray<EncLambdaInfo> lambdaDebugInfo,
ImmutableArray<LambdaRuntimeRudeEditInfo> orderedLambdaRuntimeRudeEdits,
ImmutableArray<EncClosureInfo> closureDebugInfo,
ImmutableArray<StateMachineStateDebugInfo> stateMachineStateDebugInfos,
StateMachineTypeSymbol stateMachineTypeOpt,
VariableSlotAllocator variableSlotAllocatorOpt,
Expand Down Expand Up @@ -1589,6 +1605,7 @@ private static MethodBody GenerateMethodBody(
builder.HasDynamicLocal,
importScopeOpt,
lambdaDebugInfo,
orderedLambdaRuntimeRudeEdits,
closureDebugInfo,
stateMachineTypeOpt?.Name,
stateMachineHoistedLocalScopes,
Expand Down
7 changes: 5 additions & 2 deletions src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpDefinitionMap.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,12 @@ protected override bool TryParseDisplayClassOrLambdaName(
out int suffixIndex,
out char idSeparator,
out bool isDisplayClass,
out bool isDisplayClassParentField,
out bool hasDebugIds)
{
suffixIndex = 0;
isDisplayClass = false;
isDisplayClassParentField = false;
hasDebugIds = false;
idSeparator = GeneratedNameConstants.IdSeparator;

Expand All @@ -205,7 +207,7 @@ protected override bool TryParseDisplayClassOrLambdaName(
return false;
}

if (generatedKind is not (GeneratedNameKind.LambdaDisplayClass or GeneratedNameKind.LambdaMethod or GeneratedNameKind.LocalFunction))
if (generatedKind is not (GeneratedNameKind.LambdaDisplayClass or GeneratedNameKind.LambdaMethod or GeneratedNameKind.LocalFunction or GeneratedNameKind.DisplayClassLocalOrField))
{
return false;
}
Expand All @@ -214,9 +216,10 @@ protected override bool TryParseDisplayClassOrLambdaName(
Debug.Assert(name.Length >= closeBracketOffset + 1);

isDisplayClass = generatedKind == GeneratedNameKind.LambdaDisplayClass;
isDisplayClassParentField = generatedKind == GeneratedNameKind.DisplayClassLocalOrField;

suffixIndex = closeBracketOffset + 2;
hasDebugIds = name.AsSpan(suffixIndex).StartsWith(GeneratedNameConstants.SuffixSeparator.AsSpan(), StringComparison.Ordinal);
hasDebugIds = !isDisplayClassParentField && name.AsSpan(suffixIndex).StartsWith(GeneratedNameConstants.SuffixSeparator.AsSpan(), StringComparison.Ordinal);

if (hasDebugIds)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncMethodToStateMachineRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ private FieldSymbol GetAwaiterField(TypeSymbol awaiterType)
if (!_awaiterFields.TryGetValue(awaiterType, out result))
{
int slotIndex;
if (slotAllocatorOpt == null || !slotAllocatorOpt.TryGetPreviousAwaiterSlotIndex(F.ModuleBuilderOpt.Translate(awaiterType, F.Syntax, F.Diagnostics.DiagnosticBag), F.Diagnostics.DiagnosticBag, out slotIndex))
if (slotAllocator == null || !slotAllocator.TryGetPreviousAwaiterSlotIndex(F.ModuleBuilderOpt.Translate(awaiterType, F.Syntax, F.Diagnostics.DiagnosticBag), F.Diagnostics.DiagnosticBag, out slotIndex))
{
slotIndex = _nextAwaiterId++;
}
Expand Down
32 changes: 25 additions & 7 deletions src/Compilers/CSharp/Portable/Lowering/ClosureConversion/ClosureConversion.Analysis.Tree.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#nullable disable

using System;
using System.Linq;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
Expand Down Expand Up @@ -134,9 +135,9 @@ public sealed class NestedFunction

public readonly ArrayBuilder<ClosureEnvironment> CapturedEnvironments
= ArrayBuilder<ClosureEnvironment>.GetInstance();

public ClosureEnvironment ContainingEnvironmentOpt;

#nullable enable
public ClosureEnvironment? ContainingEnvironmentOpt;
#nullable disable
private bool _capturesThis;

/// <summary>
Expand Down Expand Up @@ -170,16 +171,15 @@ public void Free()
}
}

[DebuggerDisplay("{GetDebuggerDisplay(), nq}")]
public sealed class ClosureEnvironment
{
public readonly SetWithInsertionOrder<Symbol> CapturedVariables;

/// <summary>
/// True if this environment captures a reference to a class environment
/// declared in a higher scope. Assigned by
/// <see cref="ComputeLambdaScopesAndFrameCaptures()"/>
/// Assigned by <see cref="ComputeLambdaScopesAndFrameCaptures()"/>.
/// </summary>
public bool CapturesParent;
public ClosureEnvironment Parent;

public readonly bool IsStruct;
internal SynthesizedClosureEnvironment SynthesizedEnvironment;
Expand All @@ -193,6 +193,24 @@ public ClosureEnvironment(IEnumerable<Symbol> capturedVariables, bool isStruct)
}
IsStruct = isStruct;
}

/// <summary>
/// True if this environment references a class environment declared in a higher scope.
/// </summary>
public bool CapturesParent => Parent != null;

private string GetDebuggerDisplay()
{
int depth = 0;
var current = Parent;
while (current != null)
{
depth++;
current = current.Parent;
}

return $"{depth}: captures [{string.Join(", ", CapturedVariables.Select(v => v.Name))}]";
}
}

/// <summary>
Expand Down
39 changes: 29 additions & 10 deletions src/Compilers/CSharp/Portable/Lowering/ClosureConversion/ClosureConversion.Analysis.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@

#nullable disable

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using Microsoft.CodeAnalysis.CodeGen;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Emit;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;

Expand All @@ -23,6 +24,7 @@ internal partial class ClosureConversion
/// </summary>
internal sealed partial class Analysis
{
#nullable enable
/// <summary>
/// If a local function is in the set, at some point in the code it is converted to a delegate and should then not be optimized to a struct closure.
/// Also contains all lambdas (as they are converted to delegates implicitly).
Expand All @@ -44,24 +46,25 @@ public bool CanTakeRefParameters(MethodSymbol function)

private readonly MethodSymbol _topLevelMethod;
private readonly int _topLevelMethodOrdinal;
private readonly VariableSlotAllocator _slotAllocatorOpt;
private readonly VariableSlotAllocator? _slotAllocator;
private readonly TypeCompilationState _compilationState;

private Analysis(
Scope scopeTree,
PooledHashSet<MethodSymbol> methodsConvertedToDelegates,
MethodSymbol topLevelMethod,
int topLevelMethodOrdinal,
VariableSlotAllocator slotAllocatorOpt,
VariableSlotAllocator? slotAllocator,
TypeCompilationState compilationState)
{
ScopeTree = scopeTree;
MethodsConvertedToDelegates = methodsConvertedToDelegates;
_topLevelMethod = topLevelMethod;
_topLevelMethodOrdinal = topLevelMethodOrdinal;
_slotAllocatorOpt = slotAllocatorOpt;
_slotAllocator = slotAllocator;
_compilationState = compilationState;
}
#nullable disable

public static Analysis Analyze(
BoundNode node,
Expand Down Expand Up @@ -175,7 +178,8 @@ private void ComputeLambdaScopesAndFrameCaptures()
if (!env.IsStruct)
{
Debug.Assert(!oldEnv.IsStruct);
oldEnv.CapturesParent = true;
Debug.Assert(oldEnv.Parent == null || oldEnv.Parent == env);
oldEnv.Parent = env;
oldEnv = env;
}
capturedEnvs.Remove(env);
Expand Down Expand Up @@ -517,16 +521,31 @@ private void MergeEnvironments()

internal DebugId GetTopLevelMethodId()
{
return _slotAllocatorOpt?.MethodId ?? new DebugId(_topLevelMethodOrdinal, _compilationState.ModuleBuilderOpt.CurrentGenerationOrdinal);
return _slotAllocator?.MethodId ?? new DebugId(_topLevelMethodOrdinal, _compilationState.ModuleBuilderOpt.CurrentGenerationOrdinal);
}

internal DebugId GetClosureId(SyntaxNode syntax, ArrayBuilder<ClosureDebugInfo> closureDebugInfo)
internal DebugId GetClosureId(ClosureEnvironment environment, SyntaxNode syntax, ArrayBuilder<EncClosureInfo> closureDebugInfo, out RuntimeRudeEdit? rudeEdit)
{
Debug.Assert(syntax != null);

var parentClosure = environment.Parent?.SynthesizedEnvironment;

// Frames are created and assigned top-down, so the parent scope's environment has to be assigned at this point.
// This may not be true if environments are merged in release build.
Debug.Assert(_slotAllocator == null || environment.Parent is null || parentClosure is not null);

rudeEdit = parentClosure?.RudeEdit;
var parentClosureId = parentClosure?.ClosureId;

var structCaptures = _slotAllocator != null && environment.IsStruct
? environment.CapturedVariables.SelectAsArray(v => v is ThisParameterSymbol ? GeneratedNames.ThisProxyFieldName() : v.Name)
: default;

DebugId closureId;
DebugId previousClosureId;
if (_slotAllocatorOpt != null && _slotAllocatorOpt.TryGetPreviousClosure(syntax, out previousClosureId))
if (rudeEdit == null &&
_slotAllocator != null &&
_slotAllocator.TryGetPreviousClosure(syntax, parentClosureId, structCaptures, out var previousClosureId, out rudeEdit) &&
rudeEdit == null)
{
closureId = previousClosureId;
}
Expand All @@ -536,7 +555,7 @@ internal DebugId GetClosureId(SyntaxNode syntax, ArrayBuilder<ClosureDebugInfo>
}

int syntaxOffset = _topLevelMethod.CalculateLocalSyntaxOffset(LambdaUtilities.GetDeclaratorPosition(syntax), syntax.SyntaxTree);
closureDebugInfo.Add(new ClosureDebugInfo(syntaxOffset, closureId));
closureDebugInfo.Add(new EncClosureInfo(new ClosureDebugInfo(syntaxOffset, closureId), parentClosureId, structCaptures));

return closureId;
}
Expand Down
Loading
Loading