Skip to content

Commit

Permalink
Use ref when possible to prevent issues with structs - fixes #634
Browse files Browse the repository at this point in the history
  • Loading branch information
GrahamTheCoder committed Oct 9, 2021
1 parent 8eb1294 commit f0629e8
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

### VB -> C#

* Convert with blocks using structs to a local ref variable[#634](https://github.com/icsharpcode/CodeConverter/issues/634)

### C# -> VB

Expand Down
7 changes: 4 additions & 3 deletions CodeConverter/CSharp/CommonConversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -540,17 +540,18 @@ public static CSSyntax.AttributeArgumentListSyntax CreateAttributeArgumentList(p
return SyntaxFactory.AttributeArgumentList(SyntaxFactory.SeparatedList(attributeArgumentSyntaxs));
}

public static CSSyntax.LocalDeclarationStatementSyntax CreateLocalVariableDeclarationAndAssignment(string variableName, ExpressionSyntax initValue)
public static CSSyntax.LocalDeclarationStatementSyntax CreateLocalVariableDeclarationAndAssignment(string variableName, ExpressionSyntax initValue, TypeSyntax optionalType = null)
{
return SyntaxFactory.LocalDeclarationStatement(CreateVariableDeclarationAndAssignment(variableName, initValue));
return SyntaxFactory.LocalDeclarationStatement(CreateVariableDeclarationAndAssignment(variableName, initValue, optionalType));
}

public static CSSyntax.VariableDeclarationSyntax CreateVariableDeclarationAndAssignment(string variableName,
ExpressionSyntax initValue, TypeSyntax explicitType = null)
{
explicitType ??= ValidSyntaxFactory.VarType;
CSSyntax.VariableDeclaratorSyntax variableDeclaratorSyntax = CreateVariableDeclarator(variableName, initValue);
var variableDeclarationSyntax = SyntaxFactory.VariableDeclaration(
explicitType ?? ValidSyntaxFactory.VarType,
explicitType,
SyntaxFactory.SingletonSeparatedList(variableDeclaratorSyntax));
return variableDeclarationSyntax;
}
Expand Down
17 changes: 10 additions & 7 deletions CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -761,20 +761,23 @@ private static CasePatternSwitchLabelSyntax VarWhen(SyntaxToken varName, Express
SyntaxFactory.WhenClause(binaryExp), SyntaxFactory.Token(SyntaxKind.ColonToken));
}

private async Task<(ExpressionSyntax Reusable, SyntaxList<StatementSyntax> Statements, ExpressionSyntax SingleUse)> GetExpressionWithoutSideEffectsAsync(VBSyntax.ExpressionSyntax vbExpr, string variableNameBase, bool forceVariable = false)
private async Task<(ExpressionSyntax Reusable, SyntaxList<StatementSyntax> Statements, ExpressionSyntax SingleUse)> GetExpressionWithoutSideEffectsAsync(VBSyntax.ExpressionSyntax vbExpr, string variableNameBase)
{
var expr = await vbExpr.AcceptAsync<ExpressionSyntax>(_expressionVisitor);
expr = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(vbExpr, expr);
SyntaxList<StatementSyntax> stmts = SyntaxFactory.List<StatementSyntax>();
ExpressionSyntax exprWithoutSideEffects;
ExpressionSyntax reusableExprWithoutSideEffects;
if (forceVariable || !await CanEvaluateMultipleTimesAsync(vbExpr)) {
var contextNode = vbExpr.GetAncestor<VBSyntax.MethodBlockBaseSyntax>() ?? (VBasic.VisualBasicSyntaxNode) vbExpr.Parent;
var varName = GetUniqueVariableNameInScope(contextNode, variableNameBase);
var stmt = CommonConversions.CreateLocalVariableDeclarationAndAssignment(varName, expr);
if (!await CanEvaluateMultipleTimesAsync(vbExpr)) {
TypeSyntax forceType = null;
if (_semanticModel.GetOperation(vbExpr.SkipIntoParens()).IsAssignableExpression()) {
forceType = SyntaxFactory.RefType(ValidSyntaxFactory.VarType);
expr = SyntaxFactory.RefExpression(expr);
}

var (stmt, id) = CreateLocalVariableWithUniqueName(vbExpr, variableNameBase, expr, forceType);
stmts = stmts.Add(stmt);
exprWithoutSideEffects = SyntaxFactory.IdentifierName(varName);
reusableExprWithoutSideEffects = exprWithoutSideEffects;
reusableExprWithoutSideEffects = exprWithoutSideEffects = id;
} else {
exprWithoutSideEffects = expr;
reusableExprWithoutSideEffects = expr.WithoutSourceMapping();
Expand Down
21 changes: 21 additions & 0 deletions CodeConverter/CSharp/OperationExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,26 @@ public static bool IsArrayElementAccess(this IOperation operation)
{
return operation != null && operation.Kind == OperationKind.ArrayElementReference;
}

public static bool IsAssignableExpression(this IOperation operation)
{
switch (operation?.Kind) {
case OperationKind.ArrayElementReference:
case OperationKind.LocalReference:
case OperationKind.ParameterReference:
case OperationKind.FieldReference:
case OperationKind.MethodReference:
case OperationKind.EventReference:
case OperationKind.InstanceReference:
case OperationKind.DynamicMemberReference:
return true;

//Just documenting since it's the only one mentioning reference that can't be assigned to AFAIK
case OperationKind.PropertyReference:
return false;
}

return false;
}
}
}
57 changes: 56 additions & 1 deletion Tests/CSharp/StatementTests/StatementTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
using System.Threading.Tasks;
using System.Reflection;
using System;
using System.Threading.Tasks;
using ICSharpCode.CodeConverter.Shared;
using ICSharpCode.CodeConverter.Tests.TestRunners;
using Newtonsoft.Json.Linq;
using Xunit;

namespace ICSharpCode.CodeConverter.Tests.CSharp.StatementTests
Expand Down Expand Up @@ -641,6 +645,57 @@ private void TestMethod()
}");
}

[Fact]
public async Task WithBlockStruct634Async()
{
await TestConversionVisualBasicToCSharpAsync(@"Imports System
Public Structure SomeStruct
Public FieldA As Integer
Public FieldB As Integer
End Structure
Module Module1
Sub Main()
Dim myArray(0) As SomeStruct
With myArray(0)
.FieldA = 3
.FieldB = 4
End With
'Outputs: FieldA was changed to New FieldA value
Console.WriteLine($""FieldA was changed to {myArray(0).FieldA}"")
Console.WriteLine($""FieldB was changed to {myArray(0).FieldB}"")
Console.ReadLine
End Sub
End Module", @"using System;
public partial struct SomeStruct
{
public int FieldA;
public int FieldB;
}
internal static partial class Module1
{
public static void Main()
{
var myArray = new SomeStruct[1];
{
ref var withBlock = ref myArray[0];
withBlock.FieldA = 3;
withBlock.FieldB = 4;
}
// Outputs: FieldA was changed to New FieldA value
Console.WriteLine($""FieldA was changed to {myArray[0].FieldA}"");
Console.WriteLine($""FieldB was changed to {myArray[0].FieldB}"");
Console.ReadLine();
}
}");
}

[Fact]
public async Task WithBlock2Async()
{
Expand Down

0 comments on commit f0629e8

Please # to comment.