提交 9501a90b 编写于 作者: A Andy Gocke 提交者: GitHub

Merge pull request #20936 from agocke/remove-old-visitors

Replaces all the functionality that the old LambdaRewriter.Analysis visitors had and uses the new Scope tree instead. The first commit replaces most of the functionality while asserting equivalence with the old visitors, and the second commit removes all the old visitors and deletes the assertions.
......@@ -8729,7 +8729,7 @@ internal class CSharpResources {
}
/// <summary>
/// Looks up a localized string similar to Instance of type &apos;{0}&apos; cannot be used inside an anonymous function, query expression, iterator block or async method.
/// Looks up a localized string similar to Instance of type &apos;{0}&apos; cannot be used inside a nested function, query expression, iterator block or async method.
/// </summary>
internal static string ERR_SpecialByRefInLambda {
get {
......
......@@ -3906,7 +3906,7 @@ You should consider suppressing the warning only if you're sure that you don't w
<value>Indexed property '{0}' must have all arguments optional</value>
</data>
<data name="ERR_SpecialByRefInLambda" xml:space="preserve">
<value>Instance of type '{0}' cannot be used inside an anonymous function, query expression, iterator block or async method</value>
<value>Instance of type '{0}' cannot be used inside a nested function, query expression, iterator block or async method</value>
</data>
<data name="ERR_SecurityAttributeMissingAction" xml:space="preserve">
<value>First argument to a security attribute must be a valid SecurityAction</value>
......
......@@ -7,7 +7,6 @@
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;
using System.Linq;
namespace Microsoft.CodeAnalysis.CSharp
{
......@@ -40,7 +39,13 @@ public sealed class Scope
/// in this scope or nested scopes. "Declared" refers to the start of the variable
/// lifetime (which, at this point in lowering, should be equivalent to lexical scope).
/// </summary>
public ArrayBuilder<Symbol> DeclaredVariables { get; } = ArrayBuilder<Symbol>.GetInstance();
/// <remarks>
/// It's important that this is a set and that enumeration order is deterministic. We loop
/// over this list to generate proxies and if we loop out of order this will cause
/// non-deterministic compilation, and if we generate duplicate proxies we'll generate
/// wasteful code in the best case and incorrect code in the worst.
/// </remarks>
public SetWithInsertionOrder<Symbol> DeclaredVariables { get; } = new SetWithInsertionOrder<Symbol>();
/// <summary>
/// The bound node representing this scope. This roughly corresponds to the bound
......@@ -54,7 +59,7 @@ public sealed class Scope
/// The closure that this scope is nested inside. Null if this scope is not nested
/// inside a closure.
/// </summary>
public Closure ContainingClosure { get; }
public Closure ContainingClosureOpt { get; }
public Scope(Scope parent, BoundNode boundNode, Closure containingClosure)
{
......@@ -62,7 +67,7 @@ public Scope(Scope parent, BoundNode boundNode, Closure containingClosure)
Parent = parent;
BoundNode = boundNode;
ContainingClosure = containingClosure;
ContainingClosureOpt = containingClosure;
}
public void Free()
......@@ -78,7 +83,6 @@ public void Free()
closure.Free();
}
Closures.Free();
DeclaredVariables.Free();
}
public override string ToString() => BoundNode.Syntax.GetText().ToString();
......@@ -118,7 +122,7 @@ public void Free()
/// Optimizes local functions such that if a local function only references other local functions
/// that capture no variables, we don't need to create capture environments for any of them.
/// </summary>
private void RemoveUnneededReferences()
private void RemoveUnneededReferences(ParameterSymbol thisParam)
{
var methodGraph = new MultiDictionary<MethodSymbol, MethodSymbol>();
var capturesThis = new HashSet<MethodSymbol>();
......@@ -132,7 +136,7 @@ private void RemoveUnneededReferences()
{
methodGraph.Add(localFunc, closure.OriginalMethodSymbol);
}
else if (capture == _topLevelMethod.ThisParameter)
else if (capture == thisParam)
{
if (capturesThis.Add(closure.OriginalMethodSymbol))
{
......@@ -168,7 +172,7 @@ private void RemoveUnneededReferences()
}
if (capturesThis.Contains(closure.OriginalMethodSymbol))
{
closure.CapturedVariables.Add(_topLevelMethod.ThisParameter);
closure.CapturedVariables.Add(thisParam);
}
});
}
......@@ -191,9 +195,10 @@ public static void VisitClosures(Scope scope, Action<Scope, Closure> action)
/// <summary>
/// Builds a tree of <see cref="Scope"/> nodes corresponding to a given method.
/// <see cref="Build(BoundNode, Analysis)"/> visits the bound tree and translates
/// information from the bound tree about variable scope, declared variables, and
/// variable captures into the resulting <see cref="Scope"/> tree.
/// <see cref="Build(BoundNode, MethodSymbol, HashSet{MethodSymbol}, DiagnosticBag)"/>
/// visits the bound tree and translates information from the bound tree about
/// variable scope, declared variables, and variable captures into the resulting
/// <see cref="Scope"/> tree.
/// </summary>
private class ScopeTreeBuilder : BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator
{
......@@ -211,24 +216,49 @@ private class ScopeTreeBuilder : BoundTreeWalkerWithStackGuardWithoutRecursionOn
/// </summary>
private readonly SmallDictionary<Symbol, Scope> _localToScope = new SmallDictionary<Symbol, Scope>();
private readonly Analysis _analysis;
private readonly MethodSymbol _topLevelMethod;
private ScopeTreeBuilder(Scope rootScope, Analysis analysis)
/// <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).
/// </summary>
private readonly HashSet<MethodSymbol> _methodsConvertedToDelegates;
private readonly DiagnosticBag _diagnostics;
private ScopeTreeBuilder(
Scope rootScope,
MethodSymbol topLevelMethod,
HashSet<MethodSymbol> methodsConvertedToDelegates,
DiagnosticBag diagnostics)
{
Debug.Assert(rootScope != null);
Debug.Assert(analysis != null);
Debug.Assert(topLevelMethod != null);
Debug.Assert(methodsConvertedToDelegates != null);
Debug.Assert(diagnostics != null);
_currentScope = rootScope;
_analysis = analysis;
_topLevelMethod = topLevelMethod;
_methodsConvertedToDelegates = methodsConvertedToDelegates;
_diagnostics = diagnostics;
}
public static Scope Build(BoundNode node, Analysis analysis)
public static Scope Build(
BoundNode node,
MethodSymbol topLevelMethod,
HashSet<MethodSymbol> methodsConvertedToDelegates,
DiagnosticBag diagnostics)
{
// This should be the top-level node
Debug.Assert(node == FindNodeToAnalyze(node));
Debug.Assert(topLevelMethod != null);
var rootScope = new Scope(parent: null, boundNode: node, containingClosure: null);
var builder = new ScopeTreeBuilder(rootScope, analysis);
var builder = new ScopeTreeBuilder(
rootScope,
topLevelMethod,
methodsConvertedToDelegates,
diagnostics);
builder.Build();
return rootScope;
}
......@@ -236,7 +266,7 @@ public static Scope Build(BoundNode node, Analysis analysis)
private void Build()
{
// Set up the current method locals
DeclareLocals(_currentScope, _analysis._topLevelMethod.Parameters);
DeclareLocals(_currentScope, _topLevelMethod.Parameters);
Visit(_currentScope.BoundNode);
}
......@@ -245,12 +275,6 @@ public override BoundNode VisitMethodGroup(BoundMethodGroup node)
public override BoundNode VisitBlock(BoundBlock node)
{
if (node.Locals.IsDefaultOrEmpty)
{
// Skip introducing a new scope if there are no new locals
return base.VisitBlock(node);
}
var oldScope = _currentScope;
_currentScope = CreateOrReuseScope(node, node.Locals);
var result = base.VisitBlock(node);
......@@ -278,12 +302,6 @@ public override BoundNode VisitSequence(BoundSequence node)
public override BoundNode VisitSwitchStatement(BoundSwitchStatement node)
{
if (node.InnerLocals.IsDefaultOrEmpty)
{
// Skip introducing a new scope if there are no new locals
return base.VisitSwitchStatement(node);
}
var oldScope = _currentScope;
_currentScope = CreateOrReuseScope(node, node.InnerLocals);
var result = base.VisitSwitchStatement(node);
......@@ -296,7 +314,7 @@ public override BoundNode VisitLambda(BoundLambda node)
var oldInExpressionTree = _inExpressionTree;
_inExpressionTree |= node.Type.IsExpressionTree();
_analysis.MethodsConvertedToDelegates.Add(node.Symbol.OriginalDefinition);
_methodsConvertedToDelegates.Add(node.Symbol.OriginalDefinition);
var result = VisitClosure(node.Symbol, node.Body);
_inExpressionTree = oldInExpressionTree;
......@@ -311,7 +329,7 @@ public override BoundNode VisitCall(BoundCall node)
if (node.Method.MethodKind == MethodKind.LocalFunction)
{
// Use OriginalDefinition to strip generic type parameters
AddIfCaptured(node.Method.OriginalDefinition);
AddIfCaptured(node.Method.OriginalDefinition, node.Syntax);
}
return base.VisitCall(node);
}
......@@ -322,36 +340,36 @@ public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationE
{
// Use OriginalDefinition to strip generic type parameters
var method = node.MethodOpt.OriginalDefinition;
AddIfCaptured(method);
_analysis.MethodsConvertedToDelegates.Add(method);
AddIfCaptured(method, node.Syntax);
_methodsConvertedToDelegates.Add(method);
}
return base.VisitDelegateCreationExpression(node);
}
public override BoundNode VisitParameter(BoundParameter node)
{
AddIfCaptured(node.ParameterSymbol);
AddIfCaptured(node.ParameterSymbol, node.Syntax);
return base.VisitParameter(node);
}
public override BoundNode VisitLocal(BoundLocal node)
{
AddIfCaptured(node.LocalSymbol);
AddIfCaptured(node.LocalSymbol, node.Syntax);
return base.VisitLocal(node);
}
public override BoundNode VisitBaseReference(BoundBaseReference node)
{
AddIfCaptured(_analysis._topLevelMethod.ThisParameter);
AddIfCaptured(_topLevelMethod.ThisParameter, node.Syntax);
return base.VisitBaseReference(node);
}
public override BoundNode VisitThisReference(BoundThisReference node)
{
var thisParam = _analysis._topLevelMethod.ThisParameter;
var thisParam = _topLevelMethod.ThisParameter;
if (thisParam != null)
{
AddIfCaptured(thisParam);
AddIfCaptured(thisParam, node.Syntax);
}
else
{
......@@ -402,8 +420,13 @@ private BoundNode VisitClosure(MethodSymbol closureSymbol, BoundBlock body)
return result;
}
private void AddIfCaptured(Symbol symbol)
private void AddIfCaptured(Symbol symbol, SyntaxNode syntax)
{
Debug.Assert(
symbol.Kind == SymbolKind.Local ||
symbol.Kind == SymbolKind.Parameter ||
symbol.Kind == SymbolKind.Method);
if (_currentClosure == null)
{
// Can't be captured if we're not in a closure
......@@ -418,6 +441,9 @@ private void AddIfCaptured(Symbol symbol)
if (symbol.ContainingSymbol != _currentClosure.OriginalMethodSymbol)
{
// Restricted types can't be hoisted, so they are not permitted to be captured
AddDiagnosticIfRestrictedType(symbol, syntax);
// Record the captured variable where it's captured
var scope = _currentScope;
var closure = _currentClosure;
......@@ -426,11 +452,11 @@ private void AddIfCaptured(Symbol symbol)
closure.CapturedVariables.Add(symbol);
// Also mark captured in enclosing scopes
while (scope.ContainingClosure == closure)
while (scope.ContainingClosureOpt == closure)
{
scope = scope.Parent;
}
closure = scope.ContainingClosure;
closure = scope.ContainingClosureOpt;
}
// Also record where the captured variable lives
......@@ -461,6 +487,33 @@ private void AddIfCaptured(Symbol symbol)
}
}
/// <summary>
/// Add a diagnostic if the type of a captured variable is a restricted type
/// </summary>
private void AddDiagnosticIfRestrictedType(Symbol capturedVariable, SyntaxNode syntax)
{
TypeSymbol type;
switch (capturedVariable.Kind)
{
case SymbolKind.Local:
type = ((LocalSymbol)capturedVariable).Type;
break;
case SymbolKind.Parameter:
type = ((ParameterSymbol)capturedVariable).Type;
break;
default:
// This should only be called for captured variables, and captured
// variables must be a method, parameter, or local symbol
Debug.Assert(capturedVariable.Kind == SymbolKind.Method);
return;
}
if (type.IsRestrictedType() == true)
{
_diagnostics.Add(ErrorCode.ERR_SpecialByRefInLambda, syntax.Location, type);
}
}
/// <summary>
/// Create a new nested scope under the current scope, or reuse the current
/// scope if there's no change in the bound node for the nested scope.
......@@ -469,14 +522,20 @@ private void AddIfCaptured(Symbol symbol)
private Scope CreateOrReuseScope<TSymbol>(BoundNode node, ImmutableArray<TSymbol> locals)
where TSymbol : Symbol
{
// We should never create a new scope with the same bound
// node. We can get into this situation for methods and
// closures where a new scope is created to add parameters
// and a new scope would be created for the method block,
// despite the fact that they should be the same scope.
var scope = _currentScope.BoundNode == node
? _currentScope
: CreateNestedScope(node, _currentScope, _currentClosure);
Scope scope;
if (locals.IsEmpty || _currentScope.BoundNode == node)
{
// We should never create a new scope with the same bound
// node. We can get into this situation for methods and
// closures where a new scope is created to add parameters
// and a new scope would be created for the method block,
// despite the fact that they should be the same scope.
scope = _currentScope;
}
else
{
scope = CreateNestedScope(node, _currentScope, _currentClosure);
}
DeclareLocals(scope, locals);
return scope;
}
......
......@@ -149,6 +149,15 @@ public MappedLocalFunction(SynthesizedLambdaMethod symbol, ClosureKind closureKi
/// </summary>
private ArrayBuilder<TypeCompilationState.MethodWithBody> _synthesizedMethods;
/// <summary>
/// TODO(https://github.com/dotnet/roslyn/projects/26): Delete this.
/// This should only be used by <see cref="NeedsProxy(Symbol)"/> which
/// hasn't had logic to move the proxy analysis into <see cref="Analysis"/>,
/// where the <see cref="Analysis.ScopeTree"/> could be walked to build
/// the proxy list.
/// </summary>
private readonly ImmutableHashSet<Symbol> _allCapturedVariables;
private LambdaRewriter(
Analysis analysis,
NamedTypeSymbol thisType,
......@@ -182,13 +191,20 @@ public MappedLocalFunction(SynthesizedLambdaMethod symbol, ClosureKind closureKi
_framePointers[thisType] = thisParameterOpt;
_seenBaseCall = method.MethodKind != MethodKind.Constructor; // only used for ctors
_synthesizedFieldNameIdDispenser = 1;
var allCapturedVars = ImmutableHashSet.CreateBuilder<Symbol>();
Analysis.VisitClosures(analysis.ScopeTree, (scope, closure) =>
{
allCapturedVars.UnionWith(closure.CapturedVariables);
});
_allCapturedVariables = allCapturedVars.ToImmutable();
}
protected override bool NeedsProxy(Symbol localOrParameter)
{
Debug.Assert(localOrParameter is LocalSymbol || localOrParameter is ParameterSymbol ||
(localOrParameter as MethodSymbol)?.MethodKind == MethodKind.LocalFunction);
return _analysis.CapturedVariables.ContainsKey(localOrParameter);
return _allCapturedVariables.Contains(localOrParameter);
}
/// <summary>
......@@ -227,16 +243,7 @@ protected override bool NeedsProxy(Symbol localOrParameter)
Debug.Assert(((object)thisParameter == null) || (thisParameter.Type == thisType));
Debug.Assert(compilationState.ModuleBuilderOpt != null);
var analysis = Analysis.Analyze(loweredBody, method);
if (!analysis.SeenLambda)
{
// Unreachable anonymous functions are ignored by the analyzer.
// No closures or lambda methods are generated.
// E.g.
// int y = 0;
// var b = false && (from z in new X(y) select f(z + y))
return loweredBody;
}
var analysis = Analysis.Analyze(loweredBody, method, diagnostics);
CheckLocalsDefined(loweredBody);
var rewriter = new LambdaRewriter(
......@@ -252,7 +259,7 @@ protected override bool NeedsProxy(Symbol localOrParameter)
diagnostics,
assignLocals);
analysis.ComputeLambdaScopesAndFrameCaptures();
analysis.ComputeLambdaScopesAndFrameCaptures(thisParameter);
rewriter.MakeFrames(closureDebugInfoBuilder);
// First, lower everything but references (calls, delegate conversions)
......@@ -341,7 +348,8 @@ private void MakeFrames(ArrayBuilder<ClosureDebugInfo> closureDebugInfo)
foreach (var captured in capturedVars)
{
if (!_analysis.VariableScope.TryGetValue(captured, out BoundNode captureScope))
var declarationScope = Analysis.GetVariableDeclarationScope(scope, captured);
if (declarationScope == null)
{
continue;
}
......@@ -357,22 +365,13 @@ private void MakeFrames(ArrayBuilder<ClosureDebugInfo> closureDebugInfo)
continue;
}
LambdaFrame frame = GetFrameForScope(captureScope, closureDebugInfo);
LambdaFrame frame = GetFrameForScope(declarationScope, closureDebugInfo);
if (captured.Kind != SymbolKind.Method && !proxies.ContainsKey(captured))
{
var hoistedField = LambdaCapturedVariable.Create(frame, captured, ref _synthesizedFieldNameIdDispenser);
proxies.Add(captured, new CapturedToFrameSymbolReplacement(hoistedField, isReusable: false));
CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(frame, hoistedField);
if (hoistedField.Type.IsRestrictedType())
{
foreach (CSharpSyntaxNode syntax in _analysis.CapturedVariables[captured])
{
// CS4013: Instance of type '{0}' cannot be used inside an anonymous function, query expression, iterator block or async method
this.Diagnostics.Add(ErrorCode.ERR_SpecialByRefInLambda, syntax.Location, hoistedField.Type);
}
}
}
}
});
......@@ -450,26 +449,27 @@ private void MakeFrames(ArrayBuilder<ClosureDebugInfo> closureDebugInfo)
return result;
}
private LambdaFrame GetFrameForScope(BoundNode scope, ArrayBuilder<ClosureDebugInfo> closureDebugInfo)
private LambdaFrame GetFrameForScope(Analysis.Scope scope, ArrayBuilder<ClosureDebugInfo> closureDebugInfo)
{
var scopeBoundNode = scope.BoundNode;
LambdaFrame frame;
if (!_frames.TryGetValue(scope, out frame))
if (!_frames.TryGetValue(scopeBoundNode, out frame))
{
var syntax = scope.Syntax;
var syntax = scopeBoundNode.Syntax;
Debug.Assert(syntax != null);
DebugId methodId = GetTopLevelMethodId();
DebugId closureId = GetClosureId(syntax, closureDebugInfo);
var canBeStruct = !_analysis.ScopesThatCantBeStructs.Contains(scope);
var canBeStruct = !_analysis.ScopesThatCantBeStructs.Contains(scopeBoundNode);
var containingMethod = _analysis.ScopeOwner[scope];
var containingMethod = scope.ContainingClosureOpt?.OriginalMethodSymbol ?? _topLevelMethod;
if (_substitutedSourceMethod != null && containingMethod == _topLevelMethod)
{
containingMethod = _substitutedSourceMethod;
}
frame = new LambdaFrame(_topLevelMethod, containingMethod, canBeStruct, syntax, methodId, closureId);
_frames.Add(scope, frame);
_frames.Add(scopeBoundNode, frame);
CompilationState.ModuleBuilderOpt.AddSynthesizedDefinition(ContainingType, frame);
if (frame.Constructor != null)
......@@ -701,14 +701,9 @@ private BoundNode IntroduceFrame(BoundNode node, LambdaFrame frame, Func<ArrayBu
// Capture any parameters of this block. This would typically occur
// at the top level of a method or lambda with captured parameters.
// TODO: speed up the following by computing it in analysis.
foreach (var variable in _analysis.CapturedVariables.Keys)
var scope = Analysis.GetScopeWithMatchingBoundNode(_analysis.ScopeTree, node);
foreach (var variable in scope.DeclaredVariables)
{
BoundNode varNode;
if (!_analysis.VariableScope.TryGetValue(variable, out varNode) || varNode != node)
{
continue;
}
InitVariableProxy(syntax, variable, framePointer, prologue);
}
......@@ -1360,7 +1355,7 @@ private DebugId GetLambdaId(SyntaxNode syntax, ClosureKind closureKind, int clos
// strictly need `this`.
if (_analysis.CanTakeRefParameters(node.Symbol))
{
lambdaScope = _analysis.ScopeParent[node.Body];
lambdaScope = Analysis.GetScopeParent(_analysis.ScopeTree, node.Body)?.BoundNode;
_ = _frames.TryGetValue(lambdaScope, out containerAsFrame);
structClosures = GetStructClosures(containerAsFrame, lambdaScope);
}
......@@ -1468,14 +1463,19 @@ private ImmutableArray<TypeSymbol> GetStructClosures(LambdaFrame containerAsFram
bool FindParentFrame(ref LambdaFrame container, ref BoundNode scope)
{
while (_analysis.ScopeParent.TryGetValue(scope, out scope))
while (true)
{
scope = Analysis.GetScopeParent(_analysis.ScopeTree, scope)?.BoundNode;
if (scope == null)
{
return false;
}
if (_frames.TryGetValue(scope, out container))
{
return true;
}
}
return false;
}
}
......@@ -1553,7 +1553,7 @@ private BoundNode RewriteLambdaConversion(BoundLambda node)
// NOTE: We require "lambdaScope != null".
// We do not want to introduce a field into an actual user's class (not a synthetic frame).
var shouldCacheInLoop = lambdaScope != null &&
lambdaScope != _analysis.ScopeParent[node.Body] &&
lambdaScope != Analysis.GetScopeParent(_analysis.ScopeTree, node.Body).BoundNode &&
InLoopOrLambda(node.Syntax, lambdaScope.Syntax);
if (shouldCacheForStaticMethod || shouldCacheInLoop)
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册