Skip to content

Commit af1c601

Browse files
GrahamTheCoderGrahamTheCoder
committed
Widen the hoisting concept
1 parent 6dab5a0 commit af1c601

9 files changed

Lines changed: 194 additions & 120 deletions

‎CodeConverter/CSharp/AdditionalAssignment.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
using Microsoft.CodeAnalysis.CSharp.Syntax;
2+
3+
namespace ICSharpCode.CodeConverter.CSharp
4+
{
5+
internal class AdditionalAssignment : IHoistedNode
6+
{
7+
public AdditionalAssignment(IdentifierNameSyntax identifierName, ExpressionSyntax expression)
8+
{
9+
IdentifierName = identifierName ?? throw new System.ArgumentNullException(nameof(identifierName));
10+
Expression = expression ?? throw new System.ArgumentNullException(nameof(expression));
11+
}
12+
13+
public ExpressionSyntax Expression { get; set; }
14+
public IdentifierNameSyntax IdentifierName { get; }
15+
}
16+
}

‎CodeConverter/CSharp/AdditionalDeclaration.cs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using Microsoft.CodeAnalysis;
5+
using Microsoft.CodeAnalysis.CSharp;
6+
using Microsoft.CodeAnalysis.CSharp.Syntax;
7+
8+
namespace ICSharpCode.CodeConverter.CSharp
9+
{
10+
internal class AdditionalDeclaration : IHoistedNode
11+
{
12+
public string Prefix { get; }
13+
public string Id { get; }
14+
public ExpressionSyntax Initializer { get; }
15+
public TypeSyntax Type { get; }
16+
17+
public AdditionalDeclaration(string prefix, ExpressionSyntax initializer, TypeSyntax type)
18+
{
19+
Prefix = prefix;
20+
Id = $"ph{Guid.NewGuid().ToString("N")}";
21+
Initializer = initializer;
22+
Type = type;
23+
}
24+
25+
public IdentifierNameSyntax IdentifierName => SyntaxFactory.IdentifierName(Id).WithAdditionalAnnotations(AdditionalLocals.Annotation);
26+
27+
28+
public static IEnumerable<StatementSyntax> ReplaceNames(IEnumerable<StatementSyntax> csNodes, Dictionary<string, string> newNames)
29+
{
30+
csNodes = csNodes.Select(csNode => ReplaceNames(csNode, newNames)).ToList();
31+
return csNodes;
32+
}
33+
34+
public static T ReplaceNames<T>(T csNode, Dictionary<string, string> newNames) where T: SyntaxNode
35+
{
36+
return csNode.ReplaceNodes(csNode.GetAnnotatedNodes(AdditionalLocals.Annotation), (_, withReplaced) => {
37+
var idns = (IdentifierNameSyntax)withReplaced;
38+
if (newNames.TryGetValue(idns.Identifier.ValueText, out var newName)) {
39+
return idns.WithoutAnnotations(AdditionalLocals.Annotation).WithIdentifier(SyntaxFactory.Identifier(newName));
40+
}
41+
return idns;
42+
});
43+
}
44+
}
45+
}

‎CodeConverter/CSharp/AdditionalLocal.cs‎

Lines changed: 0 additions & 25 deletions
This file was deleted.

‎CodeConverter/CSharp/AdditionalLocals.cs‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,45 @@
11
using System.Collections;
22
using System.Collections.Generic;
3+
using System.Linq;
34
using Microsoft.CodeAnalysis;
45

56
namespace ICSharpCode.CodeConverter.CSharp
67
{
7-
public class AdditionalLocals : IEnumerable<KeyValuePair<string, AdditionalLocal>>
8+
internal class AdditionalLocals
89
{
910
public static SyntaxAnnotation Annotation = new SyntaxAnnotation("CodeconverterAdditionalLocal");
1011

11-
private readonly Stack<Dictionary<string, AdditionalLocal>> _additionalLocals;
12+
private readonly Stack<List<IHoistedNode>> _hoistedNodesPerScope;
1213

1314
public AdditionalLocals()
1415
{
15-
_additionalLocals = new Stack<Dictionary<string, AdditionalLocal>>();
16+
_hoistedNodesPerScope = new Stack<List<IHoistedNode>>();
1617
}
1718

1819
public void PushScope()
1920
{
20-
_additionalLocals.Push(new Dictionary<string, AdditionalLocal>());
21+
_hoistedNodesPerScope.Push(new List<IHoistedNode>());
2122
}
2223

2324
public void PopScope()
2425
{
25-
_additionalLocals.Pop();
26+
_hoistedNodesPerScope.Pop();
2627
}
2728

28-
public AdditionalLocal AddAdditionalLocal(AdditionalLocal additionalLocal)
29+
public T Hoist<T>(T additionalLocal) where T: IHoistedNode
2930
{
30-
_additionalLocals.Peek().Add(additionalLocal.Id, additionalLocal);
31+
_hoistedNodesPerScope.Peek().Add(additionalLocal);
3132
return additionalLocal;
3233
}
3334

34-
public IEnumerator<KeyValuePair<string, AdditionalLocal>> GetEnumerator()
35+
public IReadOnlyCollection<AdditionalDeclaration> GetDeclarations()
3536
{
36-
return _additionalLocals.Peek().GetEnumerator();
37+
return _hoistedNodesPerScope.Peek().OfType<AdditionalDeclaration>().ToArray();
3738
}
3839

39-
IEnumerator IEnumerable.GetEnumerator()
40+
public IReadOnlyCollection<AdditionalAssignment> GetPostAssignments()
4041
{
41-
return _additionalLocals.Peek().GetEnumerator();
42-
}
43-
44-
public AdditionalLocal this[string id] {
45-
get {
46-
return _additionalLocals.Peek()[id];
47-
}
42+
return _hoistedNodesPerScope.Peek().OfType<AdditionalAssignment>().ToArray();
4843
}
4944
}
5045
}

‎CodeConverter/CSharp/DeclarationNodeVisitor.cs‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,8 @@ private IEnumerable<MemberDeclarationSyntax> CreateMemberDeclarations(IReadOnlyC
490490
foreach (var f in fieldDecls) yield return f;
491491
} else
492492
{
493-
if (_additionalLocals.Count() > 0) {
493+
var additionalDeclarationInfo = _additionalLocals.GetDeclarations();
494+
if (additionalDeclarationInfo.Count() > 0) {
494495
foreach (var additionalDecl in CreateAdditionalLocalMembers(convertedModifiers, attributes, decl)) {
495496
yield return additionalDecl;
496497
}
@@ -547,26 +548,19 @@ private IEnumerable<MemberDeclarationSyntax> CreateAdditionalLocalMembers(Syntax
547548
var methodName = invocationExpressionSyntax.Expression
548549
.ChildNodes().OfType<SimpleNameSyntax>().Last();
549550
var newMethodName = $"{methodName.Identifier.ValueText}_{v.Identifier.ValueText}";
550-
var localVars = _additionalLocals.Select(l => l.Value)
551-
.Select(al =>
552-
SyntaxFactory.LocalDeclarationStatement(
553-
CommonConversions.CreateVariableDeclarationAndAssignment(al.Prefix, al.Initializer)))
554-
.Cast<StatementSyntax>().ToList();
555-
var newInitializer = v.Initializer.Value.ReplaceNodes(
556-
v.Initializer.Value.GetAnnotatedNodes(AdditionalLocals.Annotation), (an, _) => {
557-
// This should probably use a unique name like in MethodBodyVisitor - a collision is far less likely here
558-
var id = ((IdentifierNameSyntax)an).Identifier.ValueText;
559-
return SyntaxFactory.IdentifierName(_additionalLocals[id].Prefix);
560-
});
561-
var body = SyntaxFactory.Block(
562-
localVars.Concat(SyntaxFactory.SingletonList(SyntaxFactory.ReturnStatement(newInitializer))));
563-
var methodAttrs = SyntaxFactory.List<AttributeListSyntax>();
551+
var declarationInfo = _additionalLocals.GetDeclarations();
552+
553+
var localVars = declarationInfo
554+
.Select(al => CommonConversions.CreateLocalVariableDeclarationAndAssignment(al.Prefix, al.Initializer))
555+
.ToArray<StatementSyntax>();
556+
557+
// This should probably use a unique name like in MethodBodyVisitor - a collision is far less likely here
558+
var newNames = declarationInfo.ToDictionary(l => l.Id, l => l.Prefix);
559+
var newInitializer = AdditionalDeclaration.ReplaceNames(v.Initializer.Value, newNames);
560+
561+
var body = SyntaxFactory.Block(localVars.Concat(SyntaxFactory.ReturnStatement(newInitializer).Yield()));
564562
// Method calls in initializers must be static in C# - Supporting this is #281
565-
var modifiers = SyntaxFactory.TokenList(SyntaxFactory.Token(Microsoft.CodeAnalysis.CSharp.SyntaxKind.StaticKeyword));
566-
var typeConstraints = SyntaxFactory.List<TypeParameterConstraintClauseSyntax>();
567-
var parameterList = SyntaxFactory.ParameterList();
568-
var methodDecl = SyntaxFactory.MethodDeclaration(methodAttrs, modifiers, decl.Type, null,
569-
SyntaxFactory.Identifier(newMethodName), null, parameterList, typeConstraints, body, null);
563+
var methodDecl = CreateParameterlessMethod(newMethodName, decl.Type, body);
570564
yield return methodDecl;
571565

572566
var newVar =
@@ -578,6 +572,17 @@ private IEnumerable<MemberDeclarationSyntax> CreateAdditionalLocalMembers(Syntax
578572
yield return SyntaxFactory.FieldDeclaration(SyntaxFactory.List(attributes), convertedModifiers, newVarDecl);
579573
}
580574

575+
private static MethodDeclarationSyntax CreateParameterlessMethod(string newMethodName, TypeSyntax type, BlockSyntax body)
576+
{
577+
var modifiers = SyntaxFactory.TokenList(SyntaxFactory.Token(Microsoft.CodeAnalysis.CSharp.SyntaxKind.StaticKeyword));
578+
var typeConstraints = SyntaxFactory.List<TypeParameterConstraintClauseSyntax>();
579+
var parameterList = SyntaxFactory.ParameterList();
580+
var methodAttrs = SyntaxFactory.List<AttributeListSyntax>();
581+
var methodDecl = SyntaxFactory.MethodDeclaration(methodAttrs, modifiers, type, null,
582+
SyntaxFactory.Identifier(newMethodName), null, parameterList, typeConstraints, body, null);
583+
return methodDecl;
584+
}
585+
581586
private List<MethodWithHandles> GetMethodWithHandles(VBSyntax.TypeBlockSyntax parentType)
582587
{
583588
if (parentType == null || !(this._semanticModel.GetDeclaredSymbol((global::Microsoft.CodeAnalysis.SyntaxNode)parentType) is ITypeSymbol containingType)) return new List<MethodWithHandles>();

‎CodeConverter/CSharp/ExpressionNodeVisitor.cs‎

Lines changed: 59 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -408,18 +408,21 @@ public override async Task<CSharpSyntaxNode> VisitSimpleArgument(VBasic.Syntax.S
408408
SyntaxToken token = default(SyntaxToken);
409409
string argName = null;
410410
RefKind refKind = RefKind.None;
411-
AdditionalLocal local = null;
411+
AdditionalDeclaration local = null;
412412
if (symbol is IMethodSymbol methodSymbol) {
413413
var parameters = (CommonConversions.GetCsOriginalSymbolOrNull(methodSymbol.OriginalDefinition) ?? methodSymbol).GetParameters();
414414
var refType = GetRefType(node, argList, parameters, out argName, out refKind);
415415
var expression = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Expression, (ExpressionSyntax)await node.Expression.AcceptAsync(TriviaConvertingExpressionVisitor), defaultToCast: refKind != RefKind.None);
416416

417+
string prefix = $"arg{argName}";
417418
if (refType != RefConversion.Inline) {
418419
var expressionTypeInfo = _semanticModel.GetTypeInfo(node.Expression);
419420
bool useVar = expressionTypeInfo.Type?.Equals(expressionTypeInfo.ConvertedType) == true && !CommonConversions.ShouldPreferExplicitType(node.Expression, expressionTypeInfo.ConvertedType, out var _);
420421
var typeSyntax = CommonConversions.GetTypeSyntax(expressionTypeInfo.ConvertedType, useVar);
421-
string prefix = $"arg{argName}";
422-
local = _additionalLocals.AddAdditionalLocal(new AdditionalLocal(prefix, expression, typeSyntax));
422+
local = _additionalLocals.Hoist(new AdditionalDeclaration(prefix, expression, typeSyntax));
423+
}
424+
if (refType == RefConversion.PreAndPostAssignment) {
425+
_additionalLocals.Hoist(new AdditionalAssignment(local.IdentifierName, expression));
423426
}
424427
}
425428

@@ -815,6 +818,25 @@ public override async Task<CSharpSyntaxNode> VisitInvocationExpression(
815818
VBasic.Syntax.InvocationExpressionSyntax node)
816819
{
817820
var invocationSymbol = _semanticModel.GetSymbolInfo(node).ExtractBestMatch<ISymbol>();
821+
var withinLocalFunction = RequiresLocalFunction(node, invocationSymbol as IMethodSymbol);
822+
if (withinLocalFunction) {
823+
_additionalLocals.PushScope();
824+
}
825+
try {
826+
var convertedInvocation = await ConvertInvocation(node, invocationSymbol);
827+
if (withinLocalFunction) {
828+
829+
}
830+
return await ConvertInvocation(node, invocationSymbol);
831+
} finally {
832+
if (withinLocalFunction) {
833+
_additionalLocals.PopScope();
834+
}
835+
}
836+
}
837+
838+
private async Task<CSharpSyntaxNode> ConvertInvocation(VBSyntax.InvocationExpressionSyntax node, ISymbol invocationSymbol)
839+
{
818840
var expressionSymbol = _semanticModel.GetSymbolInfo(node.Expression).ExtractBestMatch<ISymbol>();
819841
var expressionReturnType =
820842
expressionSymbol?.GetReturnType() ?? _semanticModel.GetTypeInfo(node.Expression).Type;
@@ -841,7 +863,6 @@ public override async Task<CSharpSyntaxNode> VisitInvocationExpression(
841863
}
842864
//TODO: Decide if the above override should be subject to the rest of this method's adjustments (probably)
843865

844-
CreateLocalByRefFunction(node, invocationSymbol);
845866

846867
// VB doesn't have a specialized node for element access because the syntax is ambiguous. Instead, it just uses an invocation expression or dictionary access expression, then figures out using the semantic model which one is most likely intended.
847868
// https://github.com/dotnet/roslyn/blob/master/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb#L768
@@ -891,46 +912,52 @@ async Task<CSharpSyntaxNode> CreateElementAccess()
891912
}
892913
}
893914

894-
915+
895916
/// <summary>
896917
/// If we need a local function,
897918
/// </summary>
898919
/// <param name="invocation"></param>
899920
/// <param name="invocationSymbol"></param>
900921
/// <returns></returns>
901-
private async Task<(string Id, StatementSyntax FunctionDeclaration)> CreateLocalByRefFunction(VBSyntax.InvocationExpressionSyntax invocation, IMethodSymbol invocationSymbol)
902-
{
903-
var originalArgsWithRefTypes = invocation.ArgumentList.Arguments
904-
.Select(a => (Arg: (VBSyntax.SimpleArgumentSyntax)a, RefType: GetRefType((VBSyntax.SimpleArgumentSyntax)a, invocation.ArgumentList, invocationSymbol.Parameters, out var argName, out var refKind), Name: argName, RefKind: refKind))
905-
.ToArray();
922+
//private async Task<(string Id, StatementSyntax FunctionDeclaration)> CreateLocalByRefFunction(VBSyntax.InvocationExpressionSyntax invocation, IMethodSymbol invocationSymbol)
923+
//{
924+
// RequiresLocalFunction(invocation, invocationSymbol);
925+
926+
// var localFuncName = $"local{invocationSymbol.Name}";
927+
// const string retVariableName = "ret";
928+
// var localFuncId = SyntaxFactory.IdentifierName(localFuncName);
929+
// // Need essentially the original invocation with any byref args swapped out for temporaries
930+
// var callAndStoreResult = CommonConversions.CreateLocalVariableDeclarationAndAssignment(retVariableName,
931+
// SyntaxFactory.InvocationExpression(expression,
932+
// );
933+
934+
// var block = SyntaxFactory.Block(
935+
// callAndStoreResult,
936+
// AssignStmt(expression, tempArg),
937+
// SyntaxFactory.ReturnStatement(SyntaxFactory.IdentifierName(retVariableName))
938+
// );
939+
// var localfunction = SyntaxFactory.LocalFunctionStatement(CommonConversions.GetTypeSyntax(invocationSymbol.ReturnType),
940+
// localFuncId.Identifier).WithBody(block);
941+
// return (localFuncName, localfunction);
942+
//}
943+
944+
private bool RequiresLocalFunction(VBSyntax.InvocationExpressionSyntax invocation, IMethodSymbol invocationSymbol)
945+
{
946+
if (invocationSymbol == null) return false;
906947

907-
var isDefinitelyExecuted = IsDefinitelyExecutedInStatement(invocation);
908-
909-
var localFuncName = $"local{invocationSymbol.Name}";
910-
const string retVariableName = "ret";
911-
var localFuncId = SyntaxFactory.IdentifierName(localFuncName);
912-
// Need essentially the original invocation with any byref args swapped out for temporaries
913-
var callAndStoreResult = CommonConversions.CreateLocalVariableDeclarationAndAssignment(retVariableName,
914-
SyntaxFactory.InvocationExpression(expression,
915-
);
948+
var originalArgsWithRefTypes = invocation.ArgumentList.Arguments
949+
.Select(a => (Arg: (VBSyntax.SimpleArgumentSyntax)a, RefType: GetRefType((VBSyntax.SimpleArgumentSyntax)a, invocation.ArgumentList, invocationSymbol.Parameters, out var argName, out var refKind), Name: argName, RefKind: refKind));
916950

917-
var block = SyntaxFactory.Block(
918-
callAndStoreResult,
919-
AssignStmt(expression, tempArg),
920-
SyntaxFactory.ReturnStatement(SyntaxFactory.IdentifierName(retVariableName))
921-
);
922-
var localfunction = SyntaxFactory.LocalFunctionStatement(CommonConversions.GetTypeSyntax(invocationSymbol.ReturnType),
923-
localFuncId.Identifier).WithBody(block);
924-
return (localFuncName, localfunction);
951+
return originalArgsWithRefTypes.Any(x => x.RefType != RefConversion.Inline) && !IsDefinitelyExecutedInStatement(invocation);
925952
}
926953

927954
private static bool IsDefinitelyExecutedInStatement(VBSyntax.InvocationExpressionSyntax invocation)
928955
{
929-
VBSyntax.StatementSyntax parentStatement;
956+
SyntaxNode parentStatement = invocation;
930957
do {
931-
parentStatement = invocation.GetAncestor<VBSyntax.StatementSyntax>();
958+
parentStatement = parentStatement.GetAncestor<VBSyntax.StatementSyntax>();
932959
} while (parentStatement is VBSyntax.ElseIfStatementSyntax);
933-
return parentStatement.FollowProperty<SyntaxNode>(n => n.ChildNodes().FirstOrDefault()).Contains(invocation);
960+
return parentStatement.FollowProperty(n => n.ChildNodes().FirstOrDefault()).Contains(invocation);
934961
}
935962

936963
public override async Task<CSharpSyntaxNode> VisitSingleLineLambdaExpression(VBasic.Syntax.SingleLineLambdaExpressionSyntax node)
@@ -1360,12 +1387,13 @@ private IEnumerable<ArgumentSyntax> GetAdditionalRequiredArgs(ISymbol invocation
13601387
private ArgumentSyntax CreateOptionalRefArg(IParameterSymbol p)
13611388
{
13621389
string prefix = $"arg{p.Name}";
1363-
var local = _additionalLocals.AddAdditionalLocal(new AdditionalLocal(prefix, CommonConversions.Literal(p.ExplicitDefaultValue), CommonConversions.GetTypeSyntax(p.Type)));
1390+
var local = _additionalLocals.Hoist(new AdditionalDeclaration(prefix, CommonConversions.Literal(p.ExplicitDefaultValue), CommonConversions.GetTypeSyntax(p.Type)));
13641391
return (ArgumentSyntax)CommonConversions.CsSyntaxGenerator.Argument(p.Name, p.RefKind, local.IdentifierName);
13651392
}
13661393

13671394
private RefConversion NeedsVariableForArgument(VBasic.Syntax.SimpleArgumentSyntax node, RefKind refKind)
13681395
{
1396+
if (refKind == RefKind.None) return RefConversion.Inline;
13691397
bool isIdentifier = node.Expression is VBasic.Syntax.IdentifierNameSyntax;
13701398
bool isMemberAccess = node.Expression is VBasic.Syntax.MemberAccessExpressionSyntax;
13711399

0 commit comments

Comments
 (0)