Skip to content

Commit c942216

Browse files
Less eagerly extract local function
1 parent 1372eec commit c942216

2 files changed

Lines changed: 24 additions & 18 deletions

File tree

CodeConverter/CSharp/ExpressionNodeVisitor.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using ICSharpCode.CodeConverter.Shared;
66
using ICSharpCode.CodeConverter.Util;
77
using ICSharpCode.CodeConverter.Util.FromRoslyn;
8+
using ICSharpCode.CodeConverter.VB;
89
using Microsoft.CodeAnalysis;
910
using Microsoft.CodeAnalysis.CSharp;
1011
using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -827,7 +828,7 @@ public override async Task<CSharpSyntaxNode> VisitInvocationExpression(
827828
if (withinLocalFunction) {
828829
return await HoistAndCallLocalFunction(node, invocationSymbol, (ExpressionSyntax)convertedInvocation);
829830
}
830-
return await ConvertInvocation(node, invocationSymbol);
831+
return convertedInvocation;
831832
} finally {
832833
if (withinLocalFunction) {
833834
_additionalLocals.PopExpressionScope();
@@ -956,9 +957,20 @@ private static bool IsDefinitelyExecutedInStatement(VBSyntax.InvocationExpressio
956957
do {
957958
parentStatement = parentStatement.GetAncestor<VBSyntax.StatementSyntax>();
958959
} while (parentStatement is VBSyntax.ElseIfStatementSyntax);
959-
return parentStatement.FollowProperty(n => n.ChildNodes().FirstOrDefault()).Contains(invocation);
960+
return parentStatement.FollowProperty(n => GetLeftMostWithPossibleExitPoints(n)).Contains(invocation);
960961
}
961962

963+
/// <summary>
964+
/// It'd be great to use _semanticModel.AnalyzeControlFlow(invocation).ExitPoints, but that doesn't account for the possibility of exceptions
965+
/// </summary>
966+
/// <param name="n"></param>
967+
/// <returns></returns>
968+
private static SyntaxNode GetLeftMostWithPossibleExitPoints(SyntaxNode n) => n switch
969+
{
970+
VBSyntax.VariableDeclaratorSyntax vds => vds.Initializer,
971+
_ => n.ChildNodes().FirstOrDefault()
972+
};
973+
962974
public override async Task<CSharpSyntaxNode> VisitSingleLineLambdaExpression(VBasic.Syntax.SingleLineLambdaExpressionSyntax node)
963975
{
964976
IReadOnlyCollection<StatementSyntax> convertedStatements;

Tests/CSharp/ExpressionTests/ByRefTests.cs

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ Public Sub UsesRef(someBool As Boolean, someInt As Integer)
353353
TakesRefVoid(1) 'Requires variable before
354354
TakesRefVoid(Prop2) ' Requires variable before, and to assign back after
355355
356-
Dim a =TakesRef(someInt) ' Convert directly
356+
Dim a = TakesRef(someInt) ' Convert directly
357357
Dim b = TakesRef(2) 'Requires variable before
358358
Dim c = TakesRef(Prop) ' Requires variable before, and to assign back after
359359
@@ -366,7 +366,8 @@ Else If TakesRef(Prop) ' Requires variable before, and to assign back after (in
366366
End If
367367
Console.WriteLine(someInt)
368368
End Sub
369-
End Class", @"
369+
End Class", @"using System;
370+
370371
public partial class MyTestClass
371372
{
372373
private int Prop { get; set; }
@@ -389,25 +390,18 @@ public void UsesRef(bool someBool, int someInt)
389390
int argvrbTst = 1;
390391
TakesRefVoid(ref argvrbTst); // Requires variable before
391392
int argvrbTst1 = Prop2;
392-
TakesRefVoid(ref argvrbTst1); // Requires variable before, and to assign back after
393+
TakesRefVoid(ref argvrbTst1);
394+
Prop2 = argvrbTst1; // Requires variable before, and to assign back after
393395
bool a = TakesRef(ref someInt); // Convert directly
394396
int argvrbTst2 = 2;
395397
bool b = TakesRef(ref argvrbTst2); // Requires variable before
396398
int argvrbTst3 = Prop;
397-
bool c = TakesRef(ref argvrbTst3); // Requires variable before, and to assign back after
399+
bool c = TakesRef(ref argvrbTst3);
400+
Prop = argvrbTst3; // Requires variable before, and to assign back after
401+
bool localTakesRef1() { int argvrbTst = Prop; var ret = TakesRef(ref argvrbTst); Prop = argvrbTst; return ret; }
402+
403+
bool localTakesRef2() { int argvrbTst = 3; var ret = TakesRef(ref argvrbTst); return ret; }
398404
399-
bool localTakesRef1() {
400-
int argvrbTst4 = 3;
401-
var ret = TakesRef(ref argvrbTst4);
402-
return ret;
403-
}
404-
bool localTakesRef2() {
405-
int argvrbTst5 = Prop;
406-
var ret = TakesRef(ref argvrbTst5);
407-
Prop = argvrbTst5;
408-
return ret;
409-
}
410-
411405
if (16 > someInt || TakesRef(ref someInt)) // Convert directly
412406
{
413407
Console.WriteLine(1);

0 commit comments

Comments
 (0)