-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Remove RemoveUnneededReferences from LamdaRewriter #21367
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,6 +119,23 @@ public readonly ArrayBuilder<ClosureEnvironment> CapturedEnvironments | |
|
||
public ClosureEnvironment ContainingEnvironmentOpt; | ||
|
||
private bool _capturesThis; | ||
|
||
/// <summary> | ||
/// True if this closure directly or transitively captures 'this' (captures | ||
/// a local function which directly or indirectly captures 'this'). | ||
/// Calculated in <see cref="MakeAndAssignEnvironments"/>. | ||
/// </summary> | ||
public bool CapturesThis | ||
{ | ||
get => _capturesThis; | ||
set | ||
{ | ||
Debug.Assert(value); | ||
_capturesThis = value; | ||
} | ||
} | ||
|
||
public Closure(MethodSymbol symbol) | ||
{ | ||
Debug.Assert(symbol != null); | ||
|
@@ -132,81 +149,29 @@ public void Free() | |
} | ||
} | ||
|
||
/// <summary> | ||
/// 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(ParameterSymbol thisParam) | ||
public sealed class ClosureEnvironment | ||
{ | ||
var methodGraph = new MultiDictionary<MethodSymbol, MethodSymbol>(); | ||
var capturesThis = new HashSet<MethodSymbol>(); | ||
var capturesVariable = new HashSet<MethodSymbol>(); | ||
var visitStack = new Stack<MethodSymbol>(); | ||
VisitClosures(ScopeTree, (scope, closure) => | ||
{ | ||
foreach (var capture in closure.CapturedVariables) | ||
{ | ||
if (capture is MethodSymbol localFunc) | ||
{ | ||
methodGraph.Add(localFunc, closure.OriginalMethodSymbol); | ||
} | ||
else if (capture == thisParam) | ||
{ | ||
if (capturesThis.Add(closure.OriginalMethodSymbol)) | ||
{ | ||
visitStack.Push(closure.OriginalMethodSymbol); | ||
} | ||
} | ||
else if (capturesVariable.Add(closure.OriginalMethodSymbol) && | ||
!capturesThis.Contains(closure.OriginalMethodSymbol)) | ||
{ | ||
visitStack.Push(closure.OriginalMethodSymbol); | ||
} | ||
} | ||
}); | ||
|
||
while (visitStack.Count > 0) | ||
{ | ||
var current = visitStack.Pop(); | ||
var setToAddTo = capturesVariable.Contains(current) ? capturesVariable : capturesThis; | ||
foreach (var capturesCurrent in methodGraph[current]) | ||
{ | ||
if (setToAddTo.Add(capturesCurrent)) | ||
{ | ||
visitStack.Push(capturesCurrent); | ||
} | ||
} | ||
} | ||
public readonly SetWithInsertionOrder<Symbol> CapturedVariables; | ||
|
||
/// <summary> | ||
/// Represents a <see cref="SynthesizedEnvironment"/> that had its environment | ||
/// pointer (a local pointing to the environment) captured like a captured | ||
/// variable. Assigned in | ||
/// <see cref="ComputeLambdaScopesAndFrameCaptures(ParameterSymbol)"/> | ||
/// </summary> | ||
public bool CapturesParent; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this value go to |
||
|
||
// True if there are any closures in the tree which | ||
// capture 'this' and another variable | ||
bool captureMoreThanThis = false; | ||
public readonly bool IsStruct; | ||
internal SynthesizedClosureEnvironment SynthesizedEnvironment; | ||
|
||
VisitClosures(ScopeTree, (scope, closure) => | ||
public ClosureEnvironment(IEnumerable<Symbol> capturedVariables, bool isStruct) | ||
{ | ||
if (!capturesVariable.Contains(closure.OriginalMethodSymbol)) | ||
{ | ||
closure.CapturedVariables.Clear(); | ||
} | ||
|
||
if (capturesThis.Contains(closure.OriginalMethodSymbol)) | ||
CapturedVariables = new SetWithInsertionOrder<Symbol>(); | ||
foreach (var item in capturedVariables) | ||
{ | ||
closure.CapturedVariables.Add(thisParam); | ||
if (closure.CapturedVariables.Count > 1) | ||
{ | ||
captureMoreThanThis |= true; | ||
} | ||
CapturedVariables.Add(item); | ||
} | ||
}); | ||
|
||
if (!captureMoreThanThis && capturesThis.Count > 0) | ||
{ | ||
// If we have closures which capture 'this', and nothing else, we can | ||
// remove 'this' from the declared variables list, since we don't need | ||
// to create an environment to hold 'this' (since we can emit the | ||
// lowered methods directly onto the containing class) | ||
bool removed = ScopeTree.DeclaredVariables.Remove(thisParam); | ||
Debug.Assert(removed); | ||
IsStruct = isStruct; | ||
} | ||
} | ||
|
||
|
@@ -226,6 +191,31 @@ public static void VisitClosures(Scope scope, Action<Scope, Closure> action) | |
} | ||
} | ||
|
||
/// <summary> | ||
/// Visit all the closures and return true when the <paramref name="func"/> returns | ||
/// true. Otherwise, returns false. | ||
/// </summary> | ||
public static bool CheckClosures(Scope scope, Func<Scope, Closure, bool> func) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this method used? Can't find a reference in this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In |
||
{ | ||
foreach (var closure in scope.Closures) | ||
{ | ||
if (func(scope, closure)) | ||
{ | ||
return true; | ||
} | ||
} | ||
|
||
foreach (var nested in scope.NestedScopes) | ||
{ | ||
if (CheckClosures(nested, func)) | ||
{ | ||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/// <summary> | ||
/// Visit the tree with the given root and run the <paramref name="action"/> | ||
/// </summary> | ||
|
@@ -491,6 +481,13 @@ private void AddIfCaptured(Symbol symbol, SyntaxNode syntax) | |
return; | ||
} | ||
|
||
if (symbol is MethodSymbol method && | ||
_currentClosure.OriginalMethodSymbol == method) | ||
{ | ||
// Is this recursion? If so there's no capturing | ||
return; | ||
} | ||
|
||
if (symbol.ContainingSymbol != _currentClosure.OriginalMethodSymbol) | ||
{ | ||
// Restricted types can't be hoisted, so they are not permitted to be captured | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blank line