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

LambdaRewriter should only assign proxies from locals in original frame in EE #19349

Merged
merged 1 commit into from
May 18, 2017
Merged

LambdaRewriter should only assign proxies from locals in original frame in EE #19349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
124 changes: 62 additions & 62 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 @@ -649,75 +649,75 @@ private void CompileSynthesizedMethods(TypeCompilationState compilationState)
var importChain = methodWithBody.ImportChainOpt;
compilationState.CurrentImportChain = importChain;

// We make sure that an asynchronous mutation to the diagnostic bag does not
// confuse the method body generator by making a fresh bag and then loading
// any diagnostics emitted into it back into the main diagnostic bag.
var diagnosticsThisMethod = DiagnosticBag.GetInstance();
// We make sure that an asynchronous mutation to the diagnostic bag does not
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// We mak [](start = 20, length = 9)

Is this block just indented?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the only change is to indenting.

// confuse the method body generator by making a fresh bag and then loading
// any diagnostics emitted into it back into the main diagnostic bag.
var diagnosticsThisMethod = DiagnosticBag.GetInstance();

var method = methodWithBody.Method;
var lambda = method as SynthesizedLambdaMethod;
var variableSlotAllocatorOpt = ((object)lambda != null) ?
_moduleBeingBuiltOpt.TryCreateVariableSlotAllocator(lambda, lambda.TopLevelMethod, diagnosticsThisMethod) :
_moduleBeingBuiltOpt.TryCreateVariableSlotAllocator(method, method, diagnosticsThisMethod);

// Synthesized methods have no ordinal stored in custom debug information (only user-defined methods have ordinals).
// In case of async lambdas, which synthesize a state machine type during the following rewrite, the containing method has already been uniquely named,
// so there is no need to produce a unique method ordinal for the corresponding state machine type, whose name includes the (unique) containing method name.
const int methodOrdinal = -1;
MethodBody emittedBody = null;
var lambda = method as SynthesizedLambdaMethod;
var variableSlotAllocatorOpt = ((object)lambda != null) ?
_moduleBeingBuiltOpt.TryCreateVariableSlotAllocator(lambda, lambda.TopLevelMethod, diagnosticsThisMethod) :
_moduleBeingBuiltOpt.TryCreateVariableSlotAllocator(method, method, diagnosticsThisMethod);

try
{
// Local functions can be iterators as well as be async (lambdas can only be async), so we need to lower both iterators and async
IteratorStateMachine iteratorStateMachine;
BoundStatement loweredBody = IteratorRewriter.Rewrite(methodWithBody.Body, method, methodOrdinal, variableSlotAllocatorOpt, compilationState, diagnosticsThisMethod, out iteratorStateMachine);
StateMachineTypeSymbol stateMachine = iteratorStateMachine;
// Synthesized methods have no ordinal stored in custom debug information (only user-defined methods have ordinals).
// In case of async lambdas, which synthesize a state machine type during the following rewrite, the containing method has already been uniquely named,
// so there is no need to produce a unique method ordinal for the corresponding state machine type, whose name includes the (unique) containing method name.
const int methodOrdinal = -1;
MethodBody emittedBody = null;

if (!loweredBody.HasErrors)
try
{
AsyncStateMachine asyncStateMachine;
loweredBody = AsyncRewriter.Rewrite(loweredBody, method, methodOrdinal, variableSlotAllocatorOpt, compilationState, diagnosticsThisMethod, out asyncStateMachine);
// Local functions can be iterators as well as be async (lambdas can only be async), so we need to lower both iterators and async
IteratorStateMachine iteratorStateMachine;
BoundStatement loweredBody = IteratorRewriter.Rewrite(methodWithBody.Body, method, methodOrdinal, variableSlotAllocatorOpt, compilationState, diagnosticsThisMethod, out iteratorStateMachine);
StateMachineTypeSymbol stateMachine = iteratorStateMachine;

Debug.Assert(iteratorStateMachine == null || asyncStateMachine == null);
stateMachine = stateMachine ?? asyncStateMachine;
}
if (!loweredBody.HasErrors)
{
AsyncStateMachine asyncStateMachine;
loweredBody = AsyncRewriter.Rewrite(loweredBody, method, methodOrdinal, variableSlotAllocatorOpt, compilationState, diagnosticsThisMethod, out asyncStateMachine);

Debug.Assert(iteratorStateMachine == null || asyncStateMachine == null);
stateMachine = stateMachine ?? asyncStateMachine;
}

if (!diagnosticsThisMethod.HasAnyErrors() && !_globalHasErrors)
if (!diagnosticsThisMethod.HasAnyErrors() && !_globalHasErrors)
{
emittedBody = GenerateMethodBody(
_moduleBeingBuiltOpt,
method,
methodOrdinal,
loweredBody,
ImmutableArray<LambdaDebugInfo>.Empty,
ImmutableArray<ClosureDebugInfo>.Empty,
stateMachine,
variableSlotAllocatorOpt,
diagnosticsThisMethod,
_debugDocumentProvider,
method.GenerateDebugInfo ? importChain : null,
emittingPdb: _emittingPdb,
emitTestCoverageData: _emitTestCoverageData,
dynamicAnalysisSpans: ImmutableArray<SourceSpan>.Empty);
}
}
catch (BoundTreeVisitor.CancelledByStackGuardException ex)
{
emittedBody = GenerateMethodBody(
_moduleBeingBuiltOpt,
method,
methodOrdinal,
loweredBody,
ImmutableArray<LambdaDebugInfo>.Empty,
ImmutableArray<ClosureDebugInfo>.Empty,
stateMachine,
variableSlotAllocatorOpt,
diagnosticsThisMethod,
_debugDocumentProvider,
method.GenerateDebugInfo ? importChain : null,
emittingPdb: _emittingPdb,
emitTestCoverageData: _emitTestCoverageData,
dynamicAnalysisSpans: ImmutableArray<SourceSpan>.Empty);
ex.AddAnError(_diagnostics);
}
}
catch (BoundTreeVisitor.CancelledByStackGuardException ex)
{
ex.AddAnError(_diagnostics);
}

_diagnostics.AddRange(diagnosticsThisMethod);
diagnosticsThisMethod.Free();
_diagnostics.AddRange(diagnosticsThisMethod);
diagnosticsThisMethod.Free();

// error while generating IL
if (emittedBody == null)
{
break;
}
// error while generating IL
if (emittedBody == null)
{
break;
}

_moduleBeingBuiltOpt.SetMethodBody(method, emittedBody);
_moduleBeingBuiltOpt.SetMethodBody(method, emittedBody);
}
}
}
finally
{
compilationState.CurrentImportChain = oldImportChain;
Expand Down Expand Up @@ -1143,7 +1143,7 @@ private void CompileMethod(
{
if (processedInitializers.LoweredInitializers.Kind == BoundKind.StatementList)
{
BoundStatementList lowered = (BoundStatementList) processedInitializers.LoweredInitializers;
BoundStatementList lowered = (BoundStatementList)processedInitializers.LoweredInitializers;
boundStatements = boundStatements.Concat(lowered.Statements);
}
else
Expand Down Expand Up @@ -1235,7 +1235,7 @@ internal static BoundStatement LowerBodyOrInitializer(
previousSubmissionFields: previousSubmissionFields,
allowOmissionOfConditionalCalls: true,
instrumentForDynamicAnalysis: instrumentForDynamicAnalysis,
debugDocumentProvider:debugDocumentProvider,
debugDocumentProvider: debugDocumentProvider,
dynamicAnalysisSpans: ref dynamicAnalysisSpans,
diagnostics: diagnostics,
sawLambdas: out sawLambdas,
Expand Down Expand Up @@ -1288,7 +1288,7 @@ internal static BoundStatement LowerBodyOrInitializer(
lazyVariableSlotAllocator,
compilationState,
diagnostics,
assignLocals: false);
assignLocals: null);
}

if (bodyWithoutLambdas.HasErrors)
Expand Down Expand Up @@ -1359,7 +1359,7 @@ private static MethodBody GenerateMethodBody(
bool isAsyncStateMachine;
MethodSymbol kickoffMethod;

if (method is SynthesizedStateMachineMethod stateMachineMethod &&
if (method is SynthesizedStateMachineMethod stateMachineMethod &&
method.Name == WellKnownMemberNames.MoveNextMethodName)
{
kickoffMethod = stateMachineMethod.StateMachineType.KickoffMethod;
Expand Down Expand Up @@ -1387,9 +1387,9 @@ private static MethodBody GenerateMethodBody(
// So it is undesirable to consider these exceptions "user unhandled" since there may well be user code that is awaiting the task.
// This is a heuristic since it's possible that there is no user code awaiting the task.
moveNextBodyDebugInfoOpt = new AsyncMoveNextBodyDebugInfo(
kickoffMethod,
kickoffMethod,
kickoffMethod.ReturnsVoid ? asyncCatchHandlerOffset : -1,
asyncYieldPoints,
asyncYieldPoints,
asyncResumePoints);
}
else
Expand Down
18 changes: 9 additions & 9 deletions src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ public MappedLocalFunction(SynthesizedLambdaMethod symbol, ClosureKind closureKi
// or the "this" parameter when at the top level. Keys in this map are never constructed types.
private readonly Dictionary<NamedTypeSymbol, Symbol> _framePointers = new Dictionary<NamedTypeSymbol, Symbol>();

// True if the rewritten tree should include assignments of the
// original locals to the lifted proxies. This is only useful for the
// expression evaluator where the original locals are left as is.
private readonly bool _assignLocals;
// The set of original locals that should be assigned to proxies
// if lifted. This is useful for the expression evaluator where
// the original locals are left as is.
private readonly HashSet<LocalSymbol> _assignLocals;

// The current method or lambda being processed.
private MethodSymbol _currentMethod;
Expand Down Expand Up @@ -159,7 +159,7 @@ private LambdaRewriter(
VariableSlotAllocator slotAllocatorOpt,
TypeCompilationState compilationState,
DiagnosticBag diagnostics,
bool assignLocals)
HashSet<LocalSymbol> assignLocals)
: base(slotAllocatorOpt, compilationState, diagnostics)
{
Debug.Assert(analysis != null);
Expand Down Expand Up @@ -207,7 +207,7 @@ protected override bool NeedsProxy(Symbol localOrParameter)
/// <param name="slotAllocatorOpt">Slot allocator.</param>
/// <param name="compilationState">The caller's buffer into which we produce additional methods to be emitted by the caller</param>
/// <param name="diagnostics">Diagnostic bag for diagnostics</param>
/// <param name="assignLocals">The rewritten tree should include assignments of the original locals to the lifted proxies</param>
/// <param name="assignLocals">The set of original locals that should be assigned to proxies if lifted</param>
public static BoundStatement Rewrite(
BoundStatement loweredBody,
NamedTypeSymbol thisType,
Expand All @@ -220,7 +220,7 @@ public static BoundStatement Rewrite(
VariableSlotAllocator slotAllocatorOpt,
TypeCompilationState compilationState,
DiagnosticBag diagnostics,
bool assignLocals)
HashSet<LocalSymbol> assignLocals)
{
Debug.Assert((object)thisType != null);
Debug.Assert(((object)thisParameter == null) || (thisParameter.Type == thisType));
Expand Down Expand Up @@ -748,12 +748,12 @@ private void InitVariableProxy(SyntaxNode syntax, Symbol symbol, LocalSymbol fra
break;

case SymbolKind.Local:
if (!_assignLocals)
var local = (LocalSymbol)symbol;
if (_assignLocals == null || !_assignLocals.Contains(local))
{
return;
}

var local = (LocalSymbol)symbol;
LocalSymbol localToUse;
if (!localMap.TryGetValue(local, out localToUse))
{
Expand Down
12 changes: 5 additions & 7 deletions src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,9 @@
using System.Diagnostics;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Text;
using Microsoft.CodeAnalysis.CodeGen;
using Microsoft.CodeAnalysis.CSharp.Emit;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Symbols;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;

Expand Down Expand Up @@ -174,12 +171,13 @@ private void CheckCurrentType()
{
Debug.Assert((object)TopLevelMethod == null || TopLevelMethod.ContainingType == CurrentType);

// In EE scenarios, lambdas are considered to be contained by the user-defined methods,
// rather than the EE-defined methods for which we are generating bound nodes. This is
// because the containing symbols are used to determine the type of the "this" parameter,
// which we need to the user-defined types.
// In EE scenarios, lambdas and local functions are considered to be contained by the
// user-defined methods, rather than the EE-defined methods for which we are generating
// bound nodes. This is because the containing symbols are used to determine the type
// of the "this" parameter, which we need to be the user-defined types.
Debug.Assert((object)CurrentMethod == null ||
CurrentMethod.MethodKind == MethodKind.AnonymousFunction ||
CurrentMethod.MethodKind == MethodKind.LocalFunction ||
CurrentMethod.ContainingType == CurrentType);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4038,7 +4038,7 @@ .locals init (Program.<>c__DisplayClass0_0 V_0, //CS$<>8__locals0
}
");
}

internal CompilationVerifier VerifyOutput(string source, string output, CSharpCompilationOptions options)
{
var comp = CreateCompilationWithMscorlib45AndCSruntime(source, options: options);
Expand Down
Loading