Skip to content

Commit 3e76d52

Browse files
GrahamTheCoderGrahamTheCoder
committed
Pull out an extra temp for element access expressions
1 parent d704de3 commit 3e76d52

File tree

4 files changed

+74
-22
lines changed

4 files changed

+74
-22
lines changed

‎CodeConverter/CSharp/DeclarationNodeVisitor.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ internal class DeclarationNodeVisitor : VBasic.VisualBasicSyntaxVisitor<Task<CSh
2929
private static readonly Type DllImportType = typeof(DllImportAttribute);
3030
private static readonly Type CharSetType = typeof(CharSet);
3131
private static readonly SyntaxToken SemicolonToken = SyntaxFactory.Token(Microsoft.CodeAnalysis.CSharp.SyntaxKind.SemicolonToken);
32-
private static readonly TypeSyntax VarType = SyntaxFactory.ParseTypeName("var");
3332
private readonly CSharpCompilation _csCompilation;
3433
private readonly SyntaxGenerator _csSyntaxGenerator;
3534
private readonly Compilation _compilation;
@@ -1081,7 +1080,7 @@ public override async Task<CSharpSyntaxNode> VisitEventBlock(VBSyntax.EventBlock
10811080
var attributes = await block.AttributeLists.SelectManyAsync(CommonConversions.ConvertAttribute);
10821081
var modifiers = CommonConversions.ConvertModifiers(block, block.Modifiers, GetMemberContext(node));
10831082

1084-
var rawType = (TypeSyntax) await (block.AsClause?.Type).AcceptAsync(_triviaConvertingExpressionVisitor) ?? VarType;
1083+
var rawType = (TypeSyntax) await (block.AsClause?.Type).AcceptAsync(_triviaConvertingExpressionVisitor) ?? ValidSyntaxFactory.VarType;
10851084

10861085
var convertedAccessors = await node.Accessors.SelectAsync(async a => await a.AcceptAsync(TriviaConvertingDeclarationVisitor));
10871086
_additionalDeclarations.Add(node, convertedAccessors.OfType<MemberDeclarationSyntax>().ToArray());

‎CodeConverter/CSharp/ExpressionNodeVisitor.cs‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -407,33 +407,40 @@ public override async Task<CSharpSyntaxNode> VisitSimpleArgument(VBasic.Syntax.S
407407
return await node.Expression.AcceptAsync(TriviaConvertingExpressionVisitor);
408408
var symbol = GetInvocationSymbol(invocation);
409409
SyntaxToken token = default(SyntaxToken);
410-
var convertedExpression = (ExpressionSyntax)await node.Expression.AcceptAsync(TriviaConvertingExpressionVisitor);
410+
var convertedArgExpression = (ExpressionSyntax)await node.Expression.AcceptAsync(TriviaConvertingExpressionVisitor);
411411
if (symbol is IMethodSymbol methodSymbol) {
412412
var parameters = (CommonConversions.GetCsOriginalSymbolOrNull(methodSymbol.OriginalDefinition) ?? methodSymbol).GetParameters();
413413
var refType = GetRefConversionType(node, argList, parameters, out var argName, out var refKind);
414-
var convertedExpressionWithoutCast = convertedExpression;
415-
convertedExpression = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Expression, convertedExpression, defaultToCast: refKind != RefKind.None);
414+
var convertedExpressionWithoutCast = convertedArgExpression;
415+
convertedArgExpression = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Expression, convertedArgExpression, defaultToCast: refKind != RefKind.None);
416416

417417
if (refType != RefConversion.Inline) {
418418
string prefix = $"arg{argName}";
419419
var expressionTypeInfo = _semanticModel.GetTypeInfo(node.Expression);
420420
bool useVar = expressionTypeInfo.Type?.Equals(expressionTypeInfo.ConvertedType) == true && !CommonConversions.ShouldPreferExplicitType(node.Expression, expressionTypeInfo.ConvertedType, out var _);
421421
var typeSyntax = CommonConversions.GetTypeSyntax(expressionTypeInfo.ConvertedType, useVar);
422-
var local = _additionalLocals.Hoist(new AdditionalDeclaration(prefix, convertedExpression, typeSyntax));
422+
423+
if (convertedExpressionWithoutCast.SkipParens() is ElementAccessExpressionSyntax eae) {
424+
//Hoist out the container so we can assign back to the same one after (like VB does)
425+
var tmpContainer = _additionalLocals.Hoist(new AdditionalDeclaration("tmp", eae.Expression, ValidSyntaxFactory.VarType));
426+
convertedExpressionWithoutCast = eae.WithExpression(tmpContainer.IdentifierName);
427+
convertedArgExpression = convertedArgExpression.ReplaceNode(eae, convertedExpressionWithoutCast);
428+
}
429+
var local = _additionalLocals.Hoist(new AdditionalDeclaration(prefix, convertedArgExpression, typeSyntax));
423430

424431
if (refType == RefConversion.PreAndPostAssignment) {
425432
var convertedLocalIdentifier = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Expression, local.IdentifierName, forceSourceType: expressionTypeInfo.ConvertedType, forceTargetType: expressionTypeInfo.Type);
426433
_additionalLocals.Hoist(new AdditionalAssignment(convertedExpressionWithoutCast, convertedLocalIdentifier));
427434
}
428-
convertedExpression = local.IdentifierName;
435+
convertedArgExpression = local.IdentifierName;
429436
}
430437
token = GetRefToken(refKind);
431438
} else {
432-
convertedExpression = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Expression, convertedExpression);
439+
convertedArgExpression = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Expression, convertedArgExpression);
433440
}
434441

435442
var nameColon = node.IsNamed ? SyntaxFactory.NameColon((IdentifierNameSyntax)await node.NameColonEquals.Name.AcceptAsync(TriviaConvertingExpressionVisitor)) : null;
436-
return SyntaxFactory.Argument(nameColon, token, convertedExpression);
443+
return SyntaxFactory.Argument(nameColon, token, convertedArgExpression);
437444
}
438445

439446
private static SyntaxToken GetRefToken(RefKind refKind)

‎CodeConverter/CSharp/ValidSyntaxFactory.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ namespace ICSharpCode.CodeConverter.CSharp
1313
/// </summary>
1414
public static class ValidSyntaxFactory
1515
{
16+
public static readonly TypeSyntax VarType = SyntaxFactory.ParseTypeName("var");
17+
1618
/// <summary>
1719
/// As of VS2019 Preview 3 this is required since not passing these arguments now means "I don't want any parentheses"
1820
/// https://github.com/dotnet/roslyn/issues/33685

‎Tests/CSharp/ExpressionTests/ByRefTests.cs‎

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -427,26 +427,26 @@ public void UsesRef(bool someBool, int someInt)
427427
public async Task Issue567()
428428
{
429429
await TestConversionVisualBasicToCSharpAsync(@"Public Class Issue567
430-
Dim arr(,) As String
431-
Dim lst As List(Of String) = New List(Of String)({1.ToString(), 2.ToString(), 3.ToString()})
430+
Dim arr() As String
431+
Dim arr2(,) As String
432432
433433
Sub DoSomething(ByRef str As String)
434434
str = ""test""
435435
End Sub
436436
437437
Sub Main()
438-
DoSomething(arr(2, 2))
439-
DoSomething(lst(1))
440-
Debug.Assert(arr(2, 2) = ""test"")
438+
DoSomething(arr(1))
439+
Debug.Assert(arr(1) = ""test"")
440+
DoSomething(arr2(2, 2))
441+
Debug.Assert(arr2(2, 2) = ""test"")
441442
End Sub
442443
443-
End Class", @"using System.Collections.Generic;
444-
using System.Diagnostics;
444+
End Class", @"using System.Diagnostics;
445445
446446
public partial class Issue567
447447
{
448-
private string[,] arr;
449-
private List<string> lst = new List<string>(new[] { 1.ToString(), 2.ToString(), 3.ToString() });
448+
private string[] arr;
449+
private string[,] arr2;
450450
451451
public void DoSomething(ref string str)
452452
{
@@ -455,12 +455,56 @@ public void DoSomething(ref string str)
455455
456456
public void Main()
457457
{
458-
DoSomething(ref arr[2, 2]);
459-
string argstr = lst[1];
458+
DoSomething(ref arr[1]);
459+
Debug.Assert((arr[1] ?? """") == ""test"");
460+
DoSomething(ref arr2[2, 2]);
461+
Debug.Assert((arr2[2, 2] ?? """") == ""test"");
462+
}
463+
}");
464+
}
465+
466+
[Fact]
467+
public async Task Issue567Extended()
468+
{
469+
await TestConversionVisualBasicToCSharpAsync(@"Public Class Issue567
470+
Sub DoSomething(ByRef str As String)
471+
lst = New List(Of String)({4.ToString(), 5.ToString(), 6.ToString()})
472+
str = 999.ToString()
473+
End Sub
474+
475+
Sub Main()
476+
DoSomething(lst(1))
477+
Debug.Assert(lst(1) = 4.ToString())
478+
End Sub
479+
480+
End Class
481+
482+
Friend Module Other
483+
Public lst As List(Of String) = New List(Of String)({ 1.ToString(), 2.ToString(), 3.ToString()})
484+
End Module", @"using System.Collections.Generic;
485+
using System.Diagnostics;
486+
487+
public partial class Issue567
488+
{
489+
public void DoSomething(ref string str)
490+
{
491+
Other.lst = new List<string>(new[] { 4.ToString(), 5.ToString(), 6.ToString() });
492+
str = 999.ToString();
493+
}
494+
495+
public void Main()
496+
{
497+
var tmp = Other.lst;
498+
string argstr = tmp[1];
460499
DoSomething(ref argstr);
461-
lst[1] = argstr;
462-
Debug.Assert((arr[2, 2] ?? """") == ""test"");
500+
tmp[1] = argstr;
501+
Debug.Assert((Other.lst[1] ?? """") == (4.ToString() ?? """"));
463502
}
503+
}
504+
505+
internal static partial class Other
506+
{
507+
public static List<string> lst = new List<string>(new[] { 1.ToString(), 2.ToString(), 3.ToString() });
464508
}");
465509
}
466510
}

0 commit comments

Comments
 (0)