Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix byref parameter #204

Closed
wants to merge 4 commits into from
Closed

Fix byref parameter #204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
957b451
Declare new local variables when a literal value is passed to a byref…
Nov 12, 2018
e8a35ad
Rename some variables.
Nov 12, 2018
a99fb45
Check Kind rather than equivalence
GrahamTheCoder Nov 13, 2018
2f7f2c3
Less code
GrahamTheCoder Nov 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 53 additions & 9 deletions ICSharpCode.CodeConverter/CSharp/MethodBodyVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ private static SyntaxKind ConvertAddRemoveHandlerToCSharpSyntaxKind(VBSyntax.Add

public override SyntaxList<StatementSyntax> VisitExpressionStatement(VBSyntax.ExpressionStatementSyntax node)
{
return SingleStatement((ExpressionSyntax)node.Expression.Accept(_nodesVisitor));
return MultipleStatements((ExpressionSyntax)node.Expression.Accept(_nodesVisitor));
}

public override SyntaxList<StatementSyntax> VisitAssignmentStatement(VBSyntax.AssignmentStatementSyntax node)
Expand Down Expand Up @@ -149,13 +149,13 @@ public override SyntaxList<StatementSyntax> VisitReDimStatement(VBSyntax.ReDimSt
public override SyntaxList<StatementSyntax> VisitRedimClause(VBSyntax.RedimClauseSyntax node)
{
bool preserve = node.Parent is VBSyntax.ReDimStatementSyntax rdss && rdss.PreserveKeyword.IsKind(VBasic.SyntaxKind.PreserveKeyword);

var csTargetArrayExpression = (ExpressionSyntax) node.Expression.Accept(_nodesVisitor);
var convertedBounds = CommonConversions.ConvertArrayBounds(node.ArrayBounds).ToList();

var newArrayAssignment = CreateNewArrayAssignment(node.Expression, csTargetArrayExpression, convertedBounds, node.SpanStart);
if (!preserve) return SingleStatement(newArrayAssignment);

var oldTargetName = GetUniqueVariableNameInScope(node, "old" + csTargetArrayExpression.ToString().ToPascalCase());
var oldArrayAssignment = CreateLocalVariableDeclarationAndAssignment(oldTargetName, csTargetArrayExpression);

Expand All @@ -174,8 +174,8 @@ private IfStatementSyntax CreateConditionalArrayCopy(VBasic.VisualBasicSyntaxNod
List<ExpressionSyntax> convertedBounds)
{
var sourceLength = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, sourceArrayExpression, SyntaxFactory.IdentifierName("Length"));
var arrayCopyStatement = convertedBounds.Count == 1
? CreateArrayCopyWithMinOfLengths(sourceArrayExpression, sourceLength, targetArrayExpression, convertedBounds.Single())
var arrayCopyStatement = convertedBounds.Count == 1
? CreateArrayCopyWithMinOfLengths(sourceArrayExpression, sourceLength, targetArrayExpression, convertedBounds.Single())
: CreateArrayCopy(originalVbNode, sourceArrayExpression, sourceLength, targetArrayExpression, convertedBounds);

var oldTargetNotEqualToNull = SyntaxFactory.BinaryExpression(SyntaxKind.NotEqualsExpression, sourceArrayExpression,
Expand All @@ -187,7 +187,7 @@ private IfStatementSyntax CreateConditionalArrayCopy(VBasic.VisualBasicSyntaxNod
/// Array copy for multiple array dimensions represented by <paramref name="convertedBounds"/>
/// </summary>
/// <remarks>
/// Exception cases will sometimes silently succeed in the converted code,
/// Exception cases will sometimes silently succeed in the converted code,
/// but existing VB code relying on the exception thrown from a multidimensional redim preserve on
/// different rank arrays is hopefully rare enough that it's worth saving a few lines of code
/// </remarks>
Expand Down Expand Up @@ -417,7 +417,7 @@ public override SyntaxList<StatementSyntax> VisitForBlock(VBSyntax.ForBlockSynta
preLoopStatements.Add(loopEndDeclaration);
csToValue = SyntaxFactory.IdentifierName(loopToVariableName);
};

if (value == null) {
condition = SyntaxFactory.BinaryExpression(SyntaxKind.LessThanOrEqualExpression, id, csToValue);
} else {
Expand Down Expand Up @@ -645,7 +645,7 @@ public override SyntaxList<StatementSyntax> VisitDoLoopBlock(VBSyntax.DoLoopBloc
statements
));
}

var whileOrUntilStmt = node.LoopStatement.WhileOrUntilClause;
ExpressionSyntax conditionExpression;
bool isUntilStmt;
Expand Down Expand Up @@ -678,14 +678,58 @@ SyntaxList<StatementSyntax> SingleStatement(ExpressionSyntax expression)
{
return SyntaxFactory.SingletonList<StatementSyntax>(SyntaxFactory.ExpressionStatement(expression));
}

SyntaxList<StatementSyntax> MultipleStatements(ExpressionSyntax expression)
{
if (expression is InvocationExpressionSyntax invocationExpression) {
var statements = GetNewStatements(invocationExpression, out var newInvocationExpression);
statements.Add(SyntaxFactory.ExpressionStatement(newInvocationExpression));
return SyntaxFactory.List(statements);
}
return SyntaxFactory.SingletonList<StatementSyntax>(SyntaxFactory.ExpressionStatement(expression));
}

private static List<StatementSyntax> GetNewStatements(InvocationExpressionSyntax initialInvocationExpression, out InvocationExpressionSyntax newInvocationExpression)
{
var newDeclarations = new List<StatementSyntax>();
var newArguments = new List<ArgumentSyntax>();
var arguments = initialInvocationExpression.ArgumentList.Arguments;
for (var i = 0; i < arguments.Count; i++) {
var argumentSyntax = arguments[i];

if (IsByRefParameterWithLiteralValue(argumentSyntax)) {
var newArgumentName = $"val{i}";
newArguments.Add(
SyntaxFactory.Argument(SyntaxFactory.IdentifierName(newArgumentName))
.WithRefOrOutKeyword(SyntaxFactory.Token(SyntaxKind.RefKeyword))
);

newDeclarations.Add(
SyntaxFactory.LocalDeclarationStatement(
CreateVariableDeclaration(newArgumentName, argumentSyntax.Expression)
)
);
} else {
newArguments.Add(argumentSyntax);
}
}

newInvocationExpression = initialInvocationExpression
.WithArgumentList(SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(newArguments)));

return newDeclarations;
}

private static bool IsByRefParameterWithLiteralValue(ArgumentSyntax argumentSyntax)
=> argumentSyntax.RefOrOutKeyword.IsKind(SyntaxKind.RefKeyword) && !argumentSyntax.Expression.IsKind(SyntaxKind.IdentifierName);
}
}

static class Extensions
{
/// <summary>
/// Returns the single statement in a block if it has no nested statements.
/// If it has nested statements, and the surrounding block was removed, it could be ambiguous,
/// If it has nested statements, and the surrounding block was removed, it could be ambiguous,
/// e.g. if (...) { if (...) return null; } else return "";
/// Unbundling the middle if statement would bind the else to it, rather than the outer if statement
/// </summary>
Expand Down
30 changes: 29 additions & 1 deletion Tests/CSharp/MemberTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,34 @@ public void TestMethod<T, T2, T3>(out T argument, ref T2 argument2, T3 argument3
}");
}

[Fact]
public void TestMethodWithByRefArguments()
{
TestConversionVisualBasicToCSharpWithoutComments(
@"Class TestClass
Public Sub DoSomething(ByRef str As String, ByRef val As Boolean, ByRef dble As Double, str2 As String)
End Sub

Public Sub DoSomethingElse()
Dim dble As Double = 2
DoSomething(""sometext"", True, dble, ""sometext2"")
End Sub
End Class", @"class TestClass
{
public void DoSomething(ref string str, ref bool val, ref double dble, string str2)
{
}

public void DoSomethingElse()
{
double dble = 2;
var val0 = ""sometext"";
var val1 = true;
DoSomething(ref val0, ref val1, ref dble, ""sometext2"");
}
}");
}

[Fact]
public void TestMethodWithReturnType()
{
Expand Down Expand Up @@ -713,7 +741,7 @@ End Sub

Public Overrides Sub CreateVirtualInstance()
End Sub
End Class",
End Class",
@"internal abstract partial class TestClass1
{
public static void CreateStatic()
Expand Down