Skip to content

Commit

Permalink
Merge pull request #19349 from cston/18273
Browse files Browse the repository at this point in the history 
LambdaRewriter should only assign proxies from locals in original frame in EE
  • Loading branch information
@cston
cston authored May 18, 2017
2 parents b4d8e2c + 1935170 commit 3f402e9
Show file tree
Hide file tree
Showing 8 changed files with 296 additions and 193 deletions.
124 changes: 62 additions & 62 deletions src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
View file
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
// 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
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
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
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

0 comments on commit 3f402e9

Please sign in to comment.