From 9c3ed817810c08b27fdfe8c9cb869ffe20617977 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 17:06:27 +0000 Subject: [PATCH 01/12] Write back ByRef arguments before return, throw and yield (#896) VB copies a ByRef argument back to its property straight after the call. The converter emits that write-back after the converted statement, which after return/throw is unreachable, and after yield return only runs when the consumer resumes. When a jump statement has write-backs, evaluate its expression into a local, write back, then jump with the local. Statements without write-backs keep their single-statement form. The local uses var only when the expression's own type is the one the jump needs. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FEr8FVAmHV9bscZ82CChg --- .../MethodBodyExecutableStatementVisitor.cs | 35 +++- CodeConverter/CSharp/PerScopeState.cs | 12 ++ Tests/CSharp/ExpressionTests/ByRefTests.cs | 80 +++++++++ .../VBToCS/ByRefWriteBackPlacementTests.vb | 155 ++++++++++++++++++ 4 files changed, 279 insertions(+), 3 deletions(-) create mode 100644 Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb diff --git a/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs b/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs index 6e9720622..2d5395558 100644 --- a/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs +++ b/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs @@ -492,7 +492,8 @@ private ExpressionStatementSyntax CreateNewArrayAssignment(VBSyntax.ExpressionSy public override async Task> VisitThrowStatement(VBSyntax.ThrowStatementSyntax node) { - return SingleStatement(SyntaxFactory.ThrowStatement(await node.Expression.AcceptAsync(_expressionVisitor))); + var csExpression = await node.Expression.AcceptAsync(_expressionVisitor); + return WithWriteBacksBeforeJump(node.Expression, csExpression, "ex", SyntaxFactory.ThrowStatement); } public override async Task> VisitReturnStatement(VBSyntax.ReturnStatementSyntax node) @@ -502,7 +503,34 @@ public override async Task> VisitReturnStatement(VBS var csExpression = await node.Expression.AcceptAsync(_expressionVisitor); csExpression = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Expression, csExpression); - return SingleStatement(SyntaxFactory.ReturnStatement(csExpression)); + return WithWriteBacksBeforeJump(node.Expression, csExpression, "ret", SyntaxFactory.ReturnStatement); + } + + /// + /// VB copies a ByRef argument back to its property straight after the call, so it happens before the statement jumps away. + /// Write-backs are normally emitted after the converted statement, which here would be unreachable (or, for yield, delayed until the consumer resumes). + /// So when there are any, evaluate the expression into a local, write back, then jump with that local. + /// Without write-backs the single statement is kept. + /// + private SyntaxList WithWriteBacksBeforeJump(VBSyntax.ExpressionSyntax vbExpression, ExpressionSyntax csExpression, string resultPrefix, + Func createJump) + { + if (csExpression is null) return SingleStatement(createJump(null)); + var writeBacks = _perScopeState.TakeWriteBacks(); + if (!writeBacks.Any()) return SingleStatement(createJump(csExpression)); + + var result = _perScopeState.Hoist(new AdditionalDeclaration(resultPrefix, csExpression, GetResultTypeSyntax(vbExpression))); + return SyntaxFactory.List(writeBacks.Append(createJump(result.IdentifierName))); + } + + /// + /// var is only safe when the expression's own type is the one the jump needs: e.g. Nothing or a lambda has no natural type in C#. + /// + private TypeSyntax GetResultTypeSyntax(VBSyntax.ExpressionSyntax vbExpression) + { + var typeInfo = _semanticModel.GetTypeInfo(vbExpression); + var isNaturalType = typeInfo.Type is { } type && SymbolEqualityComparer.Default.Equals(type, typeInfo.ConvertedType); + return CommonConversions.GetTypeSyntax(typeInfo.ConvertedType, useImplicitType: isNaturalType); } public override async Task> VisitContinueStatement(VBSyntax.ContinueStatementSyntax node) @@ -513,7 +541,8 @@ public override async Task> VisitContinueStatement(V public override async Task> VisitYieldStatement(VBSyntax.YieldStatementSyntax node) { - return SingleStatement(SyntaxFactory.YieldStatement(SyntaxKind.YieldReturnStatement, await node.Expression.AcceptAsync(_expressionVisitor))); + var csExpression = await node.Expression.AcceptAsync(_expressionVisitor); + return WithWriteBacksBeforeJump(node.Expression, csExpression, "ret", e => SyntaxFactory.YieldStatement(SyntaxKind.YieldReturnStatement, e)); } public override async Task> VisitExitStatement(VBSyntax.ExitStatementSyntax node) diff --git a/CodeConverter/CSharp/PerScopeState.cs b/CodeConverter/CSharp/PerScopeState.cs index a49fff977..e49a93554 100644 --- a/CodeConverter/CSharp/PerScopeState.cs +++ b/CodeConverter/CSharp/PerScopeState.cs @@ -78,6 +78,18 @@ public IReadOnlyCollection GetDefaultInit return _hoistedNodesPerScope.Peek().OfType().ToArray(); } + /// + /// Removes the ByRef write-backs hoisted so far in the current scope, for a statement that would otherwise skip or delay them, + /// e.g. a jump statement, after which a write-back would be unreachable. + /// + public IReadOnlyCollection TakeWriteBacks() + { + var hoistedNodes = _hoistedNodesPerScope.Peek().HoistedNodes; + var writeBacks = hoistedNodes.OfType().ToArray(); + hoistedNodes.RemoveAll(n => n is AdditionalAssignment); + return writeBacks.Select(AdditionalAssignment.CreateAssignment).ToArray(); + } + private StatementSyntax[] GetPostStatements() { var scopeState = _hoistedNodesPerScope.Peek(); diff --git a/Tests/CSharp/ExpressionTests/ByRefTests.cs b/Tests/CSharp/ExpressionTests/ByRefTests.cs index 74b47b178..c3f686e9c 100644 --- a/Tests/CSharp/ExpressionTests/ByRefTests.cs +++ b/Tests/CSharp/ExpressionTests/ByRefTests.cs @@ -995,4 +995,84 @@ public void Bar() }"); } + [Fact] + public async Task WritesBackBeforeJumpingIssue896Async() + { + await TestConversionVisualBasicToCSharpAsync(@"Imports System.Collections.Generic +Public Class C + Public Property P As Integer + + Function Inc(ByRef v As Integer) As Integer + v += 1 + Return v + End Function + + Function CreateException(ByRef v As Integer) As Exception + Return New Exception() + End Function + + Function ReturnsWrittenBack() As Long + Return Inc(P) + End Function + + Function ReturnsWithoutWriteBack() As Integer + Return Inc(New Integer) + End Function + + Iterator Function YieldsWrittenBack() As IEnumerable(Of Integer) + Yield Inc(P) + End Function + + Sub ThrowsWrittenBack() + Throw CreateException(P) + End Sub +End Class", @"using System; +using System.Collections.Generic; + +public partial class C +{ + public int P { get; set; } + + public int Inc(ref int v) + { + v += 1; + return v; + } + + public Exception CreateException(ref int v) + { + return new Exception(); + } + + public long ReturnsWrittenBack() + { + int argv = P; + long ret = Inc(ref argv); + P = argv; + return ret; + } + + public int ReturnsWithoutWriteBack() + { + int argv = new int(); + return Inc(ref argv); + } + + public IEnumerable YieldsWrittenBack() + { + int argv = P; + var ret = Inc(ref argv); + P = argv; + yield return ret; + } + + public void ThrowsWrittenBack() + { + int argv = P; + var ex = CreateException(ref argv); + P = argv; + throw ex; + } +}"); + } } \ No newline at end of file diff --git a/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb b/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb new file mode 100644 index 000000000..5227537f1 --- /dev/null +++ b/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb @@ -0,0 +1,155 @@ +Imports System +Imports System.Collections.Generic +Imports System.Runtime.InteropServices +Imports Xunit + +Public Class ByRefWriteBackPlacementTests + + Public Sub ReturnWritesBackBeforeReturningIssue896() + Dim testClass As New OutParamTestClass + Assert.Equal(2, testClass.Number) + Dim result = CallSubWithOutParam(testClass) + Assert.True(result) + Assert.Equal(1, testClass.Number) + End Sub + + Private Shared Function CallSubWithOutParam(testClass As OutParamTestClass) As Boolean + Return SubWithOutParam(testClass.Number) + End Function + + Private Shared Function SubWithOutParam( ByRef out_a As Integer) As Boolean + out_a = 1 + Return True + End Function + + Private Class OutParamTestClass + Friend Property Number As Integer = 2 + End Class + + + Public Sub ReturnWithConversionWritesBackOnce() + Dim probe As New Probe + Assert.Equal(1L, ReturnAsLong(probe)) + Assert.Equal("get,set", probe.LogText) + End Sub + + Private Shared Function ReturnAsLong(probe As Probe) As Long + Return Increment(probe.Value) + End Function + + + Public Sub ReturnFromPropertyGetterWritesBack() + Dim probe As New Probe + Assert.Equal(1, probe.IncrementedValue) + Assert.Equal(1, probe.Stored) + End Sub + + + Public Sub ReturnInMultiLineLambdaWritesBack() + Dim probe As New Probe + Dim f As Func(Of Integer) = Function() + Return Increment(probe.Value) + End Function + Assert.Equal(1, f()) + Assert.Equal(1, probe.Stored) + End Sub + + + Public Sub ThrowWritesBackBeforeThrowing() + Dim probe As New Probe + Dim ex = Assert.Throws(Of InvalidOperationException)(Sub() ThrowCreated(probe)) + Assert.Equal("1", ex.Message) + Assert.Equal(1, probe.Stored) + Assert.Equal("get,set", probe.LogText) + End Sub + + Private Shared Sub ThrowCreated(probe As Probe) + Throw CreateException(probe.Value) + End Sub + + Private Shared Function CreateException(ByRef value As Integer) As Exception + value += 1 + Return New InvalidOperationException(value.ToString()) + End Function + + + Public Sub YieldWritesBackBeforeTheConsumerResumes() + Dim probe As New Probe + For Each value In YieldIncremented(probe) + Assert.Equal(value, probe.Stored) + Next + Assert.Equal(2, probe.Stored) + End Sub + + Private Shared Iterator Function YieldIncremented(probe As Probe) As IEnumerable(Of Integer) + Yield Increment(probe.Value) + Yield Increment(probe.Value) + End Function + + + Public Sub ReturnWritesBackWhenResultNameIsTaken() + Dim probe As New Probe + Assert.Equal(42, ReturnWithLocalNamedRet(probe)) + Assert.Equal(42, probe.Stored) + End Sub + + Private Shared Function ReturnWithLocalNamedRet(probe As Probe) As Integer + Dim ret = 41 + probe.Stored = ret + Return Increment(probe.Value) + End Function + + + Public Sub ReturnToWiderTypeWritesBack() + Dim probe As New Probe + Assert.Equal(CObj(1), ReturnAsObject(probe)) + Assert.Equal(1, probe.Stored) + End Sub + + Private Shared Function ReturnAsObject(probe As Probe) As Object + Return Increment(probe.Value) + End Function + + + Public Sub UnchangedStatementShapesStillWriteBack() + Dim probe As New Probe + Increment(probe.Value) + Dim local = Increment(probe.Value) + local = Increment(probe.Value) + Assert.Equal(3, local) + Assert.Equal(3, probe.Stored) + End Sub + + Private Shared Function Increment(ByRef value As Integer) As Integer + value += 1 + Return value + End Function + + Private Class Probe + Public Stored As Integer + Public ReadOnly Log As New List(Of String) + + Public ReadOnly Property LogText As String + Get + Return String.Join(",", Log) + End Get + End Property + + Public Property Value As Integer + Get + Log.Add("get") + Return Stored + End Get + Set(value As Integer) + Log.Add("set") + Stored = value + End Set + End Property + + Public ReadOnly Property IncrementedValue As Integer + Get + Return Increment(Value) + End Get + End Property + End Class +End Class From be0f178d95a3532a8974dae2aca2b1623278a7fd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 17:20:09 +0000 Subject: [PATCH 02/12] Write back ByRef arguments in block headers before the body A ByRef write-back is emitted after the converted statement. For a call in a block header (Select Case, Case, For, For Each, Using, SyncLock, With, Catch When) that statement is the whole block, so the body saw the old value, and a later Case clause wrote back even when never evaluated. These headers now take the same local-function fallback that If and ElseIf conditions already use, so the write-back happens inside the call site's own evaluation. Headers without write-backs keep their output. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FEr8FVAmHV9bscZ82CChg --- .../CSharp/NameExpressionNodeVisitor.cs | 12 +- Tests/CSharp/ExpressionTests/ByRefTests.cs | 138 ++++++++++++++++++ .../VBToCS/ByRefWriteBackPlacementTests.vb | 137 +++++++++++++++++ 3 files changed, 286 insertions(+), 1 deletion(-) diff --git a/CodeConverter/CSharp/NameExpressionNodeVisitor.cs b/CodeConverter/CSharp/NameExpressionNodeVisitor.cs index eb6de7f58..8bc25b7cc 100644 --- a/CodeConverter/CSharp/NameExpressionNodeVisitor.cs +++ b/CodeConverter/CSharp/NameExpressionNodeVisitor.cs @@ -610,7 +610,9 @@ private bool MayThrow(VBSyntax.ExpressionSyntax expression) } /// - /// Conservative version of _semanticModel.AnalyzeControlFlow(invocation).ExitPoints to account for exceptions + /// Conservative version of _semanticModel.AnalyzeControlFlow(invocation).ExitPoints to account for exceptions. + /// A block's header statement is followed by its body, so a write-back placed after the converted block would run too late + /// (or, for a later Case clause, run when the clause was never evaluated). /// private static bool NextStatementDefinitelyExecutedAfter(VBSyntax.InvocationExpressionSyntax invocation) { @@ -626,6 +628,14 @@ private static bool NextStatementDefinitelyExecutedAfter(VBSyntax.InvocationExpr case VBSyntax.IfStatementSyntax _: case VBSyntax.ElseIfStatementSyntax _: case VBSyntax.SingleLineIfStatementSyntax _: + case VBSyntax.SelectStatementSyntax _: + case VBSyntax.CaseStatementSyntax _: + case VBSyntax.ForStatementSyntax _: + case VBSyntax.ForEachStatementSyntax _: + case VBSyntax.UsingStatementSyntax _: + case VBSyntax.SyncLockStatementSyntax _: + case VBSyntax.WithStatementSyntax _: + case VBSyntax.CatchStatementSyntax _: return false; case VBSyntax.ExpressionSyntax _: case VBSyntax.StatementSyntax _: diff --git a/Tests/CSharp/ExpressionTests/ByRefTests.cs b/Tests/CSharp/ExpressionTests/ByRefTests.cs index c3f686e9c..10352be6e 100644 --- a/Tests/CSharp/ExpressionTests/ByRefTests.cs +++ b/Tests/CSharp/ExpressionTests/ByRefTests.cs @@ -1073,6 +1073,144 @@ public void ThrowsWrittenBack() P = argv; throw ex; } +}"); + } + + [Fact] + public async Task WritesBackBeforeBlockBodyAsync() + { + await TestConversionVisualBasicToCSharpAsync(@"Public Class C + Public Property P As Integer + + Function Inc(ByRef v As Integer) As Integer + v += 1 + Return v + End Function + + Function Arr(ByRef v As Integer) As Integer() + Return New Integer() {v} + End Function + + Function D(ByRef v As Integer) As IDisposable + Return Nothing + End Function + + Function IsOne(ByRef v As Integer) As Boolean + Return v = 1 + End Function + + Sub BlockHeaders() + Select Case Inc(P) + Case 1 + Console.WriteLine(1) + End Select + For i = 0 To Inc(P) + Console.WriteLine(i) + Next + For Each item In Arr(P) + Console.WriteLine(item) + Next + Using dd = D(P) + Console.WriteLine(3) + End Using + SyncLock Arr(P) + Console.WriteLine(4) + End SyncLock + With Arr(P) + Console.WriteLine(.Length) + End With + Try + Catch ex As Exception When IsOne(P) + End Try + End Sub + + Sub BlockHeaderWithoutWriteBack() + Select Case Inc(New Integer) + Case 1 + Console.WriteLine(1) + End Select + End Sub +End Class", @"using System; + +public partial class C +{ + public int P { get; set; } + + public int Inc(ref int v) + { + v += 1; + return v; + } + + public int[] Arr(ref int v) + { + return new int[] { v }; + } + + public IDisposable D(ref int v) + { + return null; + } + + public bool IsOne(ref int v) + { + return v == 1; + } + + public void BlockHeaders() + { + int localInc() { int argv = P; var ret = Inc(ref argv); P = argv; return ret; } + + switch (localInc()) + { + case 1: + Console.WriteLine(1); + break; + } + int localInc1() { int argv1 = P; var ret = Inc(ref argv1); P = argv1; return ret; } + + for (int i = 0, loopTo = localInc1(); i <= loopTo; i++) + Console.WriteLine(i); + int[] localArr() { int argv2 = P; var ret = Arr(ref argv2); P = argv2; return ret; } + + foreach (var item in localArr()) + Console.WriteLine(item); + IDisposable localD() { int argv3 = P; var ret = D(ref argv3); P = argv3; return ret; } + + using (var dd = localD()) + { + Console.WriteLine(3); + } + int[] localArr1() { int argv4 = P; var ret = Arr(ref argv4); P = argv4; return ret; } + + lock (localArr1()) + Console.WriteLine(4); + int[] localArr2() { int argv5 = P; var ret = Arr(ref argv5); P = argv5; return ret; } + + { + var withBlock = localArr2(); + Console.WriteLine(withBlock.Length); + } + bool localIsOne() { int argv6 = P; var ret = IsOne(ref argv6); P = argv6; return ret; } + + try + { + } + catch (Exception ex) when (localIsOne()) + { + } + } + + public void BlockHeaderWithoutWriteBack() + { + int argv = new int(); + switch (Inc(ref argv)) + { + case 1: + Console.WriteLine(1); + break; + } + } }"); } } \ No newline at end of file diff --git a/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb b/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb index 5227537f1..2b0f32e53 100644 --- a/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb +++ b/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb @@ -110,6 +110,143 @@ Public Class ByRefWriteBackPlacementTests Return Increment(probe.Value) End Function + + Public Sub SelectCaseBodySeesWrittenBackValue() + Dim probe As New Probe + Select Case Increment(probe.Value) + Case 1 + Assert.Equal(1, probe.Stored) + Case Else + Assert.Fail("Unexpected case") + End Select + Assert.Equal("get,set", probe.LogText) + End Sub + + + Public Sub CaseClauseWritesBackBeforeItsBody() + Dim probe As New Probe + Dim x = 1 + Select Case x + Case Increment(probe.Value) + Assert.Equal(1, probe.Stored) + Case Else + Assert.Fail("Unexpected case") + End Select + Assert.Equal("get,set", probe.LogText) + End Sub + + + Public Sub ForBoundWritesBackBeforeTheBody() + Dim probe As New Probe + Dim iterations = 0 + For i = 0 To Increment(probe.Value) + iterations += 1 + Assert.Equal(1, probe.Stored) + Next + Assert.Equal(2, iterations) + Assert.Equal("get,set", probe.LogText) + End Sub + + + Public Sub ForEachCollectionWritesBackBeforeTheBody() + Dim probe As New Probe + For Each item In New Integer() {Increment(probe.Value)} + Assert.Equal(1, probe.Stored) + Next + For Each item In Repeat(probe.Value) + Assert.Equal(2, probe.Stored) + Next + Assert.Equal(2, probe.Stored) + End Sub + + Private Shared Function Repeat(ByRef value As Integer) As Integer() + value += 1 + Return New Integer() {value} + End Function + + + Public Sub UsingResourceWritesBackBeforeTheBody() + Dim probe As New Probe + Using d = CreateDisposable(probe.Value) + Assert.Equal(1, probe.Stored) + End Using + Assert.Equal("get,set", probe.LogText) + End Sub + + Private Shared Function CreateDisposable(ByRef value As Integer) As IDisposable + value += 1 + Return New IO.MemoryStream() + End Function + + + Public Sub SyncLockWritesBackBeforeTheBody() + Dim probe As New Probe + SyncLock CreateLock(probe.Value) + Assert.Equal(1, probe.Stored) + End SyncLock + End Sub + + Private Shared Function CreateLock(ByRef value As Integer) As Object + value += 1 + Return New Object() + End Function + + + Public Sub WithWritesBackBeforeTheBody() + Dim probe As New Probe + With CreateLock(probe.Value) + Assert.Equal(1, probe.Stored) + End With + End Sub + + + Public Sub ForStartWritesBackBeforeTheBody() + Dim probe As New Probe + For i = Increment(probe.Value) To 1 + Assert.Equal(1, probe.Stored) + Next + Assert.Equal("get,set", probe.LogText) + End Sub + + + Public Sub CatchFilterWritesBackBeforeTheHandler() + Dim probe As New Probe + Try + Throw New InvalidOperationException() + Catch ex As InvalidOperationException When IncrementIsOne(probe.Value) + Assert.Equal(1, probe.Stored) + End Try + Assert.Equal("get,set", probe.LogText) + End Sub + + Private Shared Function IncrementIsOne(ByRef value As Integer) As Boolean + Return Increment(value) = 1 + End Function + + + Public Sub LaterCaseClauseIsOnlyEvaluatedWhenReached() + Dim probe As New Probe + Select Case 1 + Case 1 + Assert.Equal(0, probe.Stored) + Case Increment(probe.Value) + Assert.Fail("Unexpected case") + End Select + Assert.Equal("", probe.LogText) + End Sub + + + Public Sub NestedBlockHeadersWriteBackBeforeTheirBodies() + Dim probe As New Probe + For Each item In Repeat(probe.Value) + Select Case Increment(probe.Value) + Case 2 + Assert.Equal(2, probe.Stored) + End Select + Next + Assert.Equal(2, probe.Stored) + End Sub + Public Sub UnchangedStatementShapesStillWriteBack() Dim probe As New Probe From e5728f8f02ffa2e1b808e33fcf4b403a7552b2b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 17:24:29 +0000 Subject: [PATCH 03/12] Move write-back-before-use into PerScopeState Pure refactor, so field initializers can reuse it. No output changes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FEr8FVAmHV9bscZ82CChg --- .../MethodBodyExecutableStatementVisitor.cs | 11 ++--------- CodeConverter/CSharp/PerScopeState.cs | 16 +++++++++++++--- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs b/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs index 2d5395558..d8818142c 100644 --- a/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs +++ b/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs @@ -507,20 +507,13 @@ public override async Task> VisitReturnStatement(VBS } /// - /// VB copies a ByRef argument back to its property straight after the call, so it happens before the statement jumps away. - /// Write-backs are normally emitted after the converted statement, which here would be unreachable (or, for yield, delayed until the consumer resumes). - /// So when there are any, evaluate the expression into a local, write back, then jump with that local. - /// Without write-backs the single statement is kept. + /// Write-backs are normally emitted after the converted statement, which after a jump would be unreachable (or, for yield, delayed until the consumer resumes). /// private SyntaxList WithWriteBacksBeforeJump(VBSyntax.ExpressionSyntax vbExpression, ExpressionSyntax csExpression, string resultPrefix, Func createJump) { if (csExpression is null) return SingleStatement(createJump(null)); - var writeBacks = _perScopeState.TakeWriteBacks(); - if (!writeBacks.Any()) return SingleStatement(createJump(csExpression)); - - var result = _perScopeState.Hoist(new AdditionalDeclaration(resultPrefix, csExpression, GetResultTypeSyntax(vbExpression))); - return SyntaxFactory.List(writeBacks.Append(createJump(result.IdentifierName))); + return SyntaxFactory.List(_perScopeState.WithWriteBacksBefore(csExpression, resultPrefix, () => GetResultTypeSyntax(vbExpression), createJump)); } /// diff --git a/CodeConverter/CSharp/PerScopeState.cs b/CodeConverter/CSharp/PerScopeState.cs index e49a93554..7d7e49dc3 100644 --- a/CodeConverter/CSharp/PerScopeState.cs +++ b/CodeConverter/CSharp/PerScopeState.cs @@ -79,10 +79,20 @@ public IReadOnlyCollection GetDefaultInit } /// - /// Removes the ByRef write-backs hoisted so far in the current scope, for a statement that would otherwise skip or delay them, - /// e.g. a jump statement, after which a write-back would be unreachable. + /// VB copies a ByRef argument back to its property straight after the call, so before a statement that uses the result jumps away. + /// When there are write-backs, evaluate the expression into a local, write back, then use that local. Without any, the single statement is kept. /// - public IReadOnlyCollection TakeWriteBacks() + public IReadOnlyCollection WithWriteBacksBefore(ExpressionSyntax expression, string resultPrefix, Func getResultType, + Func createStatementUsingResult) + { + var writeBacks = TakeWriteBacks(); + if (!writeBacks.Any()) return new[] { createStatementUsingResult(expression) }; + + var result = Hoist(new AdditionalDeclaration(resultPrefix, expression, getResultType())); + return writeBacks.Append(createStatementUsingResult(result.IdentifierName)).ToArray(); + } + + private IReadOnlyCollection TakeWriteBacks() { var hoistedNodes = _hoistedNodesPerScope.Peek().HoistedNodes; var writeBacks = hoistedNodes.OfType().ToArray(); From d636135757059bdc2e03c0e78b959bb309979948 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 17:53:24 +0000 Subject: [PATCH 04/12] Keep ByRef temporaries of field initializers in a method (#1081) A ByRef argument that needs a temporary (e.g. an omitted Optional ByRef parameter, or a property) hoists a declaration and maybe a write-back. A field initializer has no statement to put them in. The old code moved them into a static method, but read v.Initializer, which is null once an instance initializer has moved to the constructor: NullReferenceException. That path also dropped write-backs, left placeholder names in the constructor and partial-class initial functions, and refused fields with several variables or nested calls. Now each field initializer is converted in its own scope. If anything was hoisted, the initializer becomes a call to a method built with the same statement machinery as method bodies: temporaries, the write-backs before returning, and local functions. The method is static when the initializer stays on the field, and an instance method when it moves to the constructor. Initializers without temporaries are unchanged. WithObjectInitializerCanReadFromPropertiesOfObjectAsync: the object initializer's temporary now gets the same unique name and explicit type as in a method body (`SomeClass @init`), instead of an unchecked `var init`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FEr8FVAmHV9bscZ82CChg --- CodeConverter/CSharp/CommonConversions.cs | 53 ++++++++++- .../CSharp/DeclarationNodeVisitor.cs | 56 +---------- CodeConverter/CSharp/HoistedFunction.cs | 7 +- CodeConverter/CSharp/PerScopeState.cs | 21 ++++ CodeConverter/CSharp/ValidSyntaxFactory.cs | 4 +- Tests/CSharp/ExpressionTests/ByRefTests.cs | 95 +++++++++++++++++++ .../CSharp/ExpressionTests/ExpressionTests.cs | 4 +- .../VBToCS/ByRefWriteBackPlacementTests.vb | 57 +++++++++++ 8 files changed, 231 insertions(+), 66 deletions(-) diff --git a/CodeConverter/CSharp/CommonConversions.cs b/CodeConverter/CSharp/CommonConversions.cs index 3f37f1985..cfc699336 100644 --- a/CodeConverter/CSharp/CommonConversions.cs +++ b/CodeConverter/CSharp/CommonConversions.cs @@ -61,6 +61,22 @@ public record VariablesDeclaration(CSSyntax.VariableDeclarationSyntax Decl, ITyp public async Task<(IReadOnlyCollection Variables, IReadOnlyCollection Methods)> SplitVariableDeclarationsAsync( VariableDeclaratorSyntax declarator, HashSet symbolsToSkip = null, bool preferExplicitType = false) + { + if (!IsField(declarator)) return await SplitVariableDeclarationsInCurrentScopeAsync(declarator, symbolsToSkip, preferExplicitType); + + // A field initializer has no statement to hoist into, so it gets its own scope, which ConvertEqualsValueClauseSyntaxAsync moves into a method + _typeContext.PerScopeState.PushScope(); + try { + return await SplitVariableDeclarationsInCurrentScopeAsync(declarator, symbolsToSkip, preferExplicitType); + } finally { + _typeContext.PerScopeState.PopScope(); + } + } + + private static bool IsField(VariableDeclaratorSyntax declarator) => declarator.Parent.IsKind(SyntaxKind.FieldDeclaration); + + private async Task<(IReadOnlyCollection Variables, IReadOnlyCollection Methods)> SplitVariableDeclarationsInCurrentScopeAsync( + VariableDeclaratorSyntax declarator, HashSet symbolsToSkip, bool preferExplicitType) { var vbInitValue = GetInitializerToConvert(declarator); var initializerOrMethodDecl = await vbInitValue.AcceptAsync(TriviaConvertingExpressionVisitor); @@ -129,7 +145,7 @@ public bool ShouldPreferExplicitType(VBSyntax.ExpressionSyntax exp, { var csTypeSyntax = GetTypeSyntax(declaredSymbolType); - bool isField = vbDeclarator.Parent.IsKind(SyntaxKind.FieldDeclaration); + bool isField = IsField(vbDeclarator); bool declaredConst = declaredSymbol is IFieldSymbol fieldSymbol && fieldSymbol.IsConst || declaredSymbol is ILocalSymbol localSymbol && localSymbol.IsConst; @@ -144,15 +160,17 @@ public bool ShouldPreferExplicitType(VBSyntax.ExpressionSyntax exp, if (isField && !declaredSymbol.IsStatic && !SemanticModel.IsDefinitelyStatic(vbName, vbInitValue)) { if (!_typeContext.Initializers.HasInstanceConstructorsOutsideThisPart) { var lhs = SyntaxFactory.IdentifierName(ConvertIdentifier(vbName.Identifier, sourceTriviaMapKind: SourceTriviaMapKind.None)); - _typeContext.Initializers.AdditionalInstanceInitializers.Add(new Assignment(lhs, CSSyntaxKind.SimpleAssignmentExpression, adjustedInitializerExpr.WithoutSourceMapping())); + var initializer = await WithHoistedNodesInMethodAsync(vbName, adjustedInitializerExpr, csTypeSyntax, isStatic: false); + _typeContext.Initializers.AdditionalInstanceInitializers.Add(new Assignment(lhs, CSSyntaxKind.SimpleAssignmentExpression, initializer.WithoutSourceMapping())); equalsValueClauseSyntax = null; } else { - var returnBlock = SyntaxFactory.Block(SyntaxFactory.ReturnStatement(adjustedInitializerExpr)); - _typeContext.PerScopeState.Hoist(new HoistedFunction(GetInitialValueFunctionName(vbName), csTypeSyntax, returnBlock, SyntaxFactory.ParameterList())); + var returnBlock = await _typeContext.PerScopeState.CreateReturnBlockAsync(vbName, adjustedInitializerExpr, csTypeSyntax, SemanticModel); + _typeContext.PerScopeState.HoistToParent(new HoistedFunction(GetInitialValueFunctionName(vbName), csTypeSyntax, returnBlock, SyntaxFactory.ParameterList())); equalsValueClauseSyntax = null; } } else { - equalsValueClauseSyntax = SyntaxFactory.EqualsValueClause(convertedInitializer); + var initializer = isField ? await WithHoistedNodesInMethodAsync(vbName, convertedInitializer, csTypeSyntax, isStatic: true) : convertedInitializer; + equalsValueClauseSyntax = SyntaxFactory.EqualsValueClause(initializer); } } else if (isField || declaredSymbol != null && SemanticModel.IsDefinitelyAssignedBeforeRead(declaredSymbol, vbName)) @@ -168,6 +186,31 @@ public bool ShouldPreferExplicitType(VBSyntax.ExpressionSyntax exp, return equalsValueClauseSyntax; } + /// + /// Temporaries for ByRef arguments (and their write-backs) need statements, which a field initializer can't contain. + /// A method keeps them together with the initializer whether it stays on the field or moves to the constructor. + /// It must be static to be called from a field initializer, and an instance method to read instance members from the constructor. + /// + private async Task WithHoistedNodesInMethodAsync(VBSyntax.ModifiedIdentifierSyntax vbName, ExpressionSyntax initializer, TypeSyntax type, bool isStatic) + { + if (!_typeContext.PerScopeState.HasHoistedNodes) return initializer; + + var body = await _typeContext.PerScopeState.CreateReturnBlockAsync(vbName, initializer, type, SemanticModel); + var methodName = GetInitializerMethodName(initializer, ConvertIdentifier(vbName.Identifier).ValueText); + _typeContext.PerScopeState.HoistToParent(new HoistedFunction(methodName, type, body, SyntaxFactory.ParameterList(), isStatic)); + return SyntaxFactory.InvocationExpression(ValidSyntaxFactory.IdentifierName(methodName)); + } + + private static string GetInitializerMethodName(ExpressionSyntax initializer, string fieldName) + { + var invokedName = initializer is CSSyntax.InvocationExpressionSyntax { Expression: var invoked } ? invoked switch { + CSSyntax.SimpleNameSyntax name => name, + CSSyntax.MemberAccessExpressionSyntax memberAccess => memberAccess.Name, + _ => null + } : null; + return invokedName is null ? "init" + fieldName.UppercaseFirstLetter() : $"{invokedName.Identifier.ValueText}_{fieldName}"; + } + /// /// In CS we need to lift non-static initializers to the constructor. But for partial classes these can be in different files. /// Rather than re-architect to allow communication between files, we create an initializer function, and call it from the other part, and just hope the name doesn't clash. diff --git a/CodeConverter/CSharp/DeclarationNodeVisitor.cs b/CodeConverter/CSharp/DeclarationNodeVisitor.cs index 74a905a13..ae3660a4d 100644 --- a/CodeConverter/CSharp/DeclarationNodeVisitor.cs +++ b/CodeConverter/CSharp/DeclarationNodeVisitor.cs @@ -553,15 +553,7 @@ private IEnumerable CreateMemberDeclarations(IReadOnlyC foreach (var f in fieldDecls) yield return f; } else { foreach (var method in CreateExtraMethodMembers()) yield return method; - - if (AdditionalLocals.GetDeclarations().Any()) { - foreach (var additionalDecl in CreateAdditionalLocalMembers(thisFieldModifiers, attributes, variablesDecl.Decl)) { - yield return additionalDecl; - } - } else { - yield return CS.SyntaxFactory.FieldDeclaration(CS.SyntaxFactory.List(attributes), thisFieldModifiers, variablesDecl.Decl); - } - + yield return CS.SyntaxFactory.FieldDeclaration(CS.SyntaxFactory.List(attributes), thisFieldModifiers, variablesDecl.Decl); } } } @@ -586,52 +578,6 @@ private IEnumerable CreateWithEventsMembers(SyntaxToken return fieldDecls; } - private IEnumerable CreateAdditionalLocalMembers(SyntaxTokenList convertedModifiers, List attributes, VariableDeclarationSyntax decl) - { - if (decl.Variables.Count > 1) { - // Currently no way to tell which _additionalLocals would apply to which initializer - throw new NotImplementedException( - "Fields with multiple declarations and initializers with ByRef parameters not currently supported"); - } - - var v = decl.Variables.First(); - var invocations = v.Initializer.Value.DescendantNodesAndSelf().OfType().ToArray(); - if (invocations.Length > 1) { - throw new NotImplementedException( - "Field initializers with nested method calls not currently supported"); - } - - string newMethodName; - if (invocations.OnlyOrDefault() is { Expression: {} e} && e.ChildNodes().OfType().LastOrDefault() is {} sns) { - newMethodName = $"{sns?.Identifier.ValueText}_{v.Identifier.ValueText}"; - } else { - newMethodName = "init" + v.Identifier.ValueText.UppercaseFirstLetter(); - } - - var declarationInfo = AdditionalLocals.GetDeclarations(); - - var localVars = declarationInfo - .Select(al => CommonConversions.CreateLocalVariableDeclarationAndAssignment(al.Prefix, al.Initializer)) - .ToArray(); - - // This should probably use a unique name like in MethodBodyVisitor - a collision is far less likely here - var newNames = declarationInfo.ToDictionary(l => l.Id, l => l.Prefix); - var newInitializer = PerScopeState.ReplaceNames(v.Initializer.Value, newNames); - - var body = CS.SyntaxFactory.Block(localVars.Concat(CS.SyntaxFactory.ReturnStatement(newInitializer).Yield())); - // Method calls in initializers must be static in C# - Supporting this is #281 - var methodDecl = ValidSyntaxFactory.CreateParameterlessMethod(newMethodName, decl.Type, body); - yield return methodDecl; - - var newVar = - v.WithInitializer(CS.SyntaxFactory.EqualsValueClause( - CS.SyntaxFactory.InvocationExpression(ValidSyntaxFactory.IdentifierName(newMethodName)))); - var newVarDecl = - CS.SyntaxFactory.VariableDeclaration(decl.Type, CS.SyntaxFactory.SingletonSeparatedList(newVar)); - - yield return CS.SyntaxFactory.FieldDeclaration(CS.SyntaxFactory.List(attributes), convertedModifiers, newVarDecl); - } - private IReadOnlyCollection CreateExtraMethodMembers() { var methodsInfos = AdditionalLocals.GetParameterlessFunctions(); diff --git a/CodeConverter/CSharp/HoistedFunction.cs b/CodeConverter/CSharp/HoistedFunction.cs index cca1c0c83..6b02f3b7d 100644 --- a/CodeConverter/CSharp/HoistedFunction.cs +++ b/CodeConverter/CSharp/HoistedFunction.cs @@ -8,20 +8,23 @@ internal class HoistedFunction : IHoistedNode private readonly TypeSyntax _returnType; private readonly BlockSyntax _block; private readonly ParameterListSyntax _parameters; + private readonly bool _isStatic; public string Id { get; } public string Prefix { get; } - public HoistedFunction(string localFuncName, TypeSyntax returnType, BlockSyntax block, ParameterListSyntax parameters) + /// Only used when hoisted to a member: it must be an instance method to be called from an instance context such as a constructor + public HoistedFunction(string localFuncName, TypeSyntax returnType, BlockSyntax block, ParameterListSyntax parameters, bool isStatic = true) { Id = $"hs{Guid.NewGuid().ToString("N")}"; Prefix = localFuncName; _returnType = returnType; _block = block; _parameters = parameters; + _isStatic = isStatic; } public IdentifierNameSyntax TempIdentifier => ValidSyntaxFactory.IdentifierName(Id).WithAdditionalAnnotations(PerScopeState.AdditionalLocalAnnotation); public LocalFunctionStatementSyntax AsLocalFunction(string functionName) => SyntaxFactory.LocalFunctionStatement(_returnType, SyntaxFactory.Identifier(functionName)).WithParameterList(_parameters).WithBody(_block); - public MethodDeclarationSyntax AsInstanceMethod(string functionName) => ValidSyntaxFactory.CreateMethod(functionName, _returnType, _parameters, _block); + public MethodDeclarationSyntax AsInstanceMethod(string functionName) => ValidSyntaxFactory.CreateMethod(functionName, _returnType, _parameters, _block, _isStatic); } \ No newline at end of file diff --git a/CodeConverter/CSharp/PerScopeState.cs b/CodeConverter/CSharp/PerScopeState.cs index 7d7e49dc3..eefdfdcb8 100644 --- a/CodeConverter/CSharp/PerScopeState.cs +++ b/CodeConverter/CSharp/PerScopeState.cs @@ -68,6 +68,27 @@ public T HoistToTopLevel(T additionalField) where T : IHoistedNode return additionalField; } + public bool HasHoistedNodes => _hoistedNodesPerScope.Peek().HoistedNodes.Any(); + + /// + /// For an expression that has no statement of its own to hoist into, e.g. a field initializer. + /// Declares the temporaries hoisted for it and writes back ByRef arguments before returning, as a statement would. + /// Leaves the current scope as it was, since each variable sharing an initializer evaluates it separately. + /// + public async Task CreateReturnBlockAsync(VBasic.VisualBasicSyntaxNode vbNode, ExpressionSyntax expression, TypeSyntax resultType, SemanticModel semanticModel) + { + var currentScope = _hoistedNodesPerScope.Peek(); + _hoistedNodesPerScope.Push(currentScope with { HoistedNodes = currentScope.HoistedNodes.ToList() }); + try { + var generatedNames = new HashSet(); + var statements = WithWriteBacksBefore(expression, "ret", () => resultType, SyntaxFactory.ReturnStatement); + var withLocals = await CreateLocalsAsync(vbNode, statements, generatedNames, semanticModel); + return SyntaxFactory.Block(CreateStatements(vbNode, withLocals, generatedNames, semanticModel)); + } finally { + PopScope(); + } + } + public IReadOnlyCollection GetDeclarations() { return _hoistedNodesPerScope.Peek().OfType().ToArray(); diff --git a/CodeConverter/CSharp/ValidSyntaxFactory.cs b/CodeConverter/CSharp/ValidSyntaxFactory.cs index 38bd3bf26..563b15de7 100644 --- a/CodeConverter/CSharp/ValidSyntaxFactory.cs +++ b/CodeConverter/CSharp/ValidSyntaxFactory.cs @@ -79,9 +79,9 @@ public static MethodDeclarationSyntax CreateParameterlessMethod(string newMethod return CreateMethod(newMethodName, type, parameterList, body); } - public static MethodDeclarationSyntax CreateMethod(string newMethodName, TypeSyntax type, ParameterListSyntax parameterList, BlockSyntax body) + public static MethodDeclarationSyntax CreateMethod(string newMethodName, TypeSyntax type, ParameterListSyntax parameterList, BlockSyntax body, bool isStatic = true) { - var modifiers = SyntaxFactory.TokenList(SyntaxFactory.Token(SyntaxKind.StaticKeyword)); + var modifiers = isStatic ? SyntaxFactory.TokenList(SyntaxFactory.Token(SyntaxKind.StaticKeyword)) : SyntaxFactory.TokenList(); var typeConstraints = SyntaxFactory.List(); var methodAttrs = SyntaxFactory.List(); diff --git a/Tests/CSharp/ExpressionTests/ByRefTests.cs b/Tests/CSharp/ExpressionTests/ByRefTests.cs index 10352be6e..24c6569b3 100644 --- a/Tests/CSharp/ExpressionTests/ByRefTests.cs +++ b/Tests/CSharp/ExpressionTests/ByRefTests.cs @@ -1211,6 +1211,101 @@ public void BlockHeaderWithoutWriteBack() break; } } +}"); + } + + [Fact] + public async Task OmittedOptionalRefInInstanceFieldInitializerIssue1081Async() + { + await TestConversionVisualBasicToCSharpAsync(@"Public Class FieldInitializationTest + Private First As New Inner() + Private Second As New Outer(First) +End Class + +Public Class Inner +End Class + +Public Class Outer + Sub New(inner As Inner, Optional ByRef x As String = """") + End Sub +End Class", @"using System.Runtime.InteropServices; + +public partial class FieldInitializationTest +{ + private Inner First = new Inner(); + + public FieldInitializationTest() + { + Second = initSecond(); + } + Outer initSecond() + { + string argx = """"; + return new Outer(First, x: ref argx); + } + + private Outer Second; +} + +public partial class Inner +{ +} + +public partial class Outer +{ + public Outer(Inner inner, [Optional, DefaultParameterValue("""")] ref string x) + { + } +}"); + } + + [Fact] + public async Task FieldInitializersWriteBackAsync() + { + await TestConversionVisualBasicToCSharpAsync(@"Public Class C + Public Property P As Integer + Private Shared Property SP As Integer + Private _a As Integer = Inc(P) + Private Shared _c As Integer = Inc(SP) + + Shared Function Inc(ByRef v As Integer) As Integer + v += 1 + Return v + End Function +End Class", @" +public partial class C +{ + public int P { get; set; } + private static int SP { get; set; } + + public C() + { + _a = Inc__a(); + } + int Inc__a() + { + int argv = P; + int ret = Inc(ref argv); + P = argv; + return ret; + } + + private int _a; + static int Inc__c() + { + int argv = SP; + int ret = Inc(ref argv); + SP = argv; + return ret; + } + + private static int _c = Inc__c(); + + public static int Inc(ref int v) + { + v += 1; + return v; + } }"); } } \ No newline at end of file diff --git a/Tests/CSharp/ExpressionTests/ExpressionTests.cs b/Tests/CSharp/ExpressionTests/ExpressionTests.cs index ef68b4387..d2bc5c561 100644 --- a/Tests/CSharp/ExpressionTests/ExpressionTests.cs +++ b/Tests/CSharp/ExpressionTests/ExpressionTests.cs @@ -1379,8 +1379,8 @@ public partial class SomeClass public string SomeProperty; static SomeClass initInstance() { - var init = new SomeClass(); - return (init.SomeProperty = init.SomeProperty + nameof(init.SomeProperty), init).init; // Second line gets moved + SomeClass @init = new SomeClass(); + return (@init.SomeProperty = @init.SomeProperty + nameof(@init.SomeProperty), @init).@init; // Second line gets moved } // Third line gets moved public static SomeClass Instance = initInstance(); // First line gets moved diff --git a/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb b/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb index 2b0f32e53..3b444e9c2 100644 --- a/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb +++ b/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb @@ -257,6 +257,63 @@ Public Class ByRefWriteBackPlacementTests Assert.Equal(3, probe.Stored) End Sub + + Public Sub InstanceFieldInitializerWithOmittedOptionalByRefIssue1081() + Dim initialized As New FieldInitializers + Assert.Same(initialized.First, initialized.Second.ReceivedInner) + Assert.Equal("", initialized.Second.ReceivedX) + End Sub + + + Public Sub InstanceFieldInitializersWriteBackInDeclarationOrder() + Dim initialized As New FieldInitializers + Assert.Equal(1, initialized.A) + Assert.Equal(22, initialized.C) + Assert.Equal(2, initialized.Counter) + End Sub + + + Public Sub EachVariableOfAFieldDeclarationGetsItsOwnTemporary() + Dim initialized As New FieldInitializers + Assert.NotSame(initialized.D, initialized.E) + Assert.Same(initialized.First, initialized.D.ReceivedInner) + Assert.Same(initialized.First, initialized.E.ReceivedInner) + End Sub + + + Public Sub SharedFieldInitializerWritesBack() + Assert.Equal(1, SharedFieldInitializer.B) + Assert.Equal(1, SharedFieldInitializer.SharedCounter) + End Sub + + Private Class FieldInitializers + Public Property Counter As Integer + Public ReadOnly First As New Inner() + Public ReadOnly Second As New Outer(First) + Public ReadOnly A As Integer = Increment(Counter) + Public ReadOnly C As Integer = Increment(Counter) + Counter * 10 + Public ReadOnly D, E As New Outer(First) + End Class + + Private Class SharedFieldInitializer + Public Shared Property SharedCounter As Integer + Public Shared ReadOnly B As Integer = Increment(SharedCounter) + End Class + + Private Class Inner + End Class + + Private Class Outer + Public ReadOnly ReceivedInner As Inner + Public ReadOnly ReceivedX As String + + Sub New(inner As Inner, Optional ByRef x As String = "") + ReceivedInner = inner + ReceivedX = x + x = "changed" + End Sub + End Class + Private Shared Function Increment(ByRef value As Integer) As Integer value += 1 Return value From 24fab8dc7ddbd7e8ad087d577e8292db4c30c8d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 01:02:26 +0000 Subject: [PATCH 05/12] Make partial-class initial value functions instance methods When a type has constructors in another part, a non-static field initializer becomes an initialX() function that the other part's constructor calls. It was always emitted static, so any initializer reading an instance member (which is why it isn't on the field) failed to compile with CS0120, including one holding ByRef temporaries. The multi-file characterization initialAnArrayWithNonStaticInitializer... read the instance field anInt from a static method; it now compiles. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FEr8FVAmHV9bscZ82CChg --- CodeConverter/CSharp/CommonConversions.cs | 2 +- .../VbLibrary/PartialClassPart.cs | 2 +- .../Prefix.VbLibrary/PartialClassPart.cs | 2 +- .../VbLibrary/PartialClassPart.cs | 2 +- .../VBToCS/ByRefWriteBackPlacementTests.vb | 17 +++++++++++++++++ 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/CodeConverter/CSharp/CommonConversions.cs b/CodeConverter/CSharp/CommonConversions.cs index cfc699336..bab4b4464 100644 --- a/CodeConverter/CSharp/CommonConversions.cs +++ b/CodeConverter/CSharp/CommonConversions.cs @@ -165,7 +165,7 @@ public bool ShouldPreferExplicitType(VBSyntax.ExpressionSyntax exp, equalsValueClauseSyntax = null; } else { var returnBlock = await _typeContext.PerScopeState.CreateReturnBlockAsync(vbName, adjustedInitializerExpr, csTypeSyntax, SemanticModel); - _typeContext.PerScopeState.HoistToParent(new HoistedFunction(GetInitialValueFunctionName(vbName), csTypeSyntax, returnBlock, SyntaxFactory.ParameterList())); + _typeContext.PerScopeState.HoistToParent(new HoistedFunction(GetInitialValueFunctionName(vbName), csTypeSyntax, returnBlock, SyntaxFactory.ParameterList(), isStatic: false)); equalsValueClauseSyntax = null; } } else { diff --git a/Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertVbLibraryOnly/VbLibrary/PartialClassPart.cs b/Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertVbLibraryOnly/VbLibrary/PartialClassPart.cs index f6a13e3a3..424a6ff69 100644 --- a/Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertVbLibraryOnly/VbLibrary/PartialClassPart.cs +++ b/Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertVbLibraryOnly/VbLibrary/PartialClassPart.cs @@ -6,7 +6,7 @@ namespace VbLibrary /// internal partial class AClass { - static int[] initialAnArrayWithNonStaticInitializerReferencingOtherPart() => new int[anInt + 1]; + int[] initialAnArrayWithNonStaticInitializerReferencingOtherPart() => new int[anInt + 1]; private int[] anArrayWithNonStaticInitializerReferencingOtherPart; } diff --git a/Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertWholeSolution/Prefix.VbLibrary/PartialClassPart.cs b/Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertWholeSolution/Prefix.VbLibrary/PartialClassPart.cs index 3f9874dde..d3e12b88a 100644 --- a/Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertWholeSolution/Prefix.VbLibrary/PartialClassPart.cs +++ b/Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertWholeSolution/Prefix.VbLibrary/PartialClassPart.cs @@ -6,7 +6,7 @@ namespace Prefix.VbLibrary /// internal partial class AClass { - static int[] initialAnArrayWithNonStaticInitializerReferencingOtherPart() => new int[anInt + 1]; + int[] initialAnArrayWithNonStaticInitializerReferencingOtherPart() => new int[anInt + 1]; private int[] anArrayWithNonStaticInitializerReferencingOtherPart; } diff --git a/Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertWholeSolution/VbLibrary/PartialClassPart.cs b/Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertWholeSolution/VbLibrary/PartialClassPart.cs index f6a13e3a3..424a6ff69 100644 --- a/Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertWholeSolution/VbLibrary/PartialClassPart.cs +++ b/Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertWholeSolution/VbLibrary/PartialClassPart.cs @@ -6,7 +6,7 @@ namespace VbLibrary /// internal partial class AClass { - static int[] initialAnArrayWithNonStaticInitializerReferencingOtherPart() => new int[anInt + 1]; + int[] initialAnArrayWithNonStaticInitializerReferencingOtherPart() => new int[anInt + 1]; private int[] anArrayWithNonStaticInitializerReferencingOtherPart; } diff --git a/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb b/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb index 3b444e9c2..c4eae72e8 100644 --- a/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb +++ b/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb @@ -314,6 +314,23 @@ Public Class ByRefWriteBackPlacementTests End Sub End Class + + Public Sub PartialClassFieldInitializerWritesBack() + Dim initialized As New PartialFieldInitializers() + Assert.Equal(1, initialized.A) + Assert.Equal(1, initialized.Counter) + End Sub + + Partial Private Class PartialFieldInitializers + Public Property Counter As Integer + Public ReadOnly A As Integer = Increment(Counter) + End Class + + Partial Private Class PartialFieldInitializers + Public Sub New() + End Sub + End Class + Private Shared Function Increment(ByRef value As Integer) As Integer value += 1 Return value From b8426735ec00334e13ccddb54e97389ffc6fddb0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 01:02:46 +0000 Subject: [PATCH 06/12] Document where ByRef temporaries and write-backs are placed Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FEr8FVAmHV9bscZ82CChg --- CodeConverter/CSharp/README.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 CodeConverter/CSharp/README.md diff --git a/CodeConverter/CSharp/README.md b/CodeConverter/CSharp/README.md new file mode 100644 index 000000000..aef66ce5f --- /dev/null +++ b/CodeConverter/CSharp/README.md @@ -0,0 +1,28 @@ +# VB → C# conversion + +## ByRef arguments that need a temporary + +Sources: [passing arguments by value and by reference](https://learn.microsoft.com/dotnet/visual-basic/programming-guide/language-features/procedures/passing-arguments-by-value-and-by-reference) and Roslyn's VB lowering in [`LocalRewriter_Call.vb`](https://github.com/dotnet/roslyn/blob/main/src/Compilers/VisualBasic/Portable/Lowering/LocalRewriter/LocalRewriter_Call.vb): `RewriteByRefArgumentWithCopyBack` creates the temporary, and `VisitCall` wraps the call in a sequence whose side effects after the value are the copy-backs. Behaviour below is covered by `Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb`. + +### VB + +* A ByRef parameter accepts any expression. If it isn't a variable (a property, a method result, a constant, an omitted `Optional` argument), VB passes a temporary. +* For a writable property (or a `ReadOnly` auto-property inside its constructor), VB copies the temporary back **straight after the call returns**, before the rest of the enclosing expression or statement runs. + +### C# + +* `ref`/`out` need a variable, so the temporary must be declared in a statement before the call. +* There is no expression-level copy-back, so a write-back is a statement too. + +### Implementation + +`ArgumentConverter.HoistByRefDeclaration` hoists the temporary (`AdditionalDeclaration`) and, when VB copies back, the write-back (`AdditionalAssignment`) into the current `PerScopeState` scope. `PerScopeState.CreateLocalsAsync` emits them before and after the converted statement. The output takes the simplest form that keeps VB's timing: + +| Where the call is | C# | Code | +|---|---|---| +| A statement, as the last thing evaluated | temporary before, write-back after the statement | `PerScopeState.CreateLocalsAsync` | +| `Return`, `Throw`, `Yield` | `var ret = call; write-back; return ret;` (only when there is a write-back) | `PerScopeState.WithWriteBacksBefore` | +| Followed by other evaluation, or in a condition or block header (`If`, `ElseIf`, loops, `Select Case`, `Case`, `For`, `For Each`, `Using`, `SyncLock`, `With`, `Catch When`) | a local function that declares, calls and writes back, called in place | `NameExpressionNodeVisitor.RequiresLocalFunction`, `HoistAndCallLocalFunctionAsync` | +| A field initializer | a method that declares, writes back and returns, called from the field (static) or the constructor (instance) | `CommonConversions.WithHoistedNodesInMethodAsync`, `PerScopeState.CreateReturnBlockAsync` | + +Known gaps: arguments of `New` (object creation) in a block header still write back after the block, and a non-constant `For ... Step` is re-evaluated each iteration. From 7377689a1617518805f4127cc5937382ad9c7d5a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 01:21:58 +0000 Subject: [PATCH 07/12] Drop unreachable case when naming a field initializer method Coverage showed the unqualified-name branch is never hit: invoked names are qualified (e.g. Me.Foo) before conversion. Falling back to init would still be a valid name. No output changes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FEr8FVAmHV9bscZ82CChg --- CodeConverter/CSharp/CommonConversions.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/CodeConverter/CSharp/CommonConversions.cs b/CodeConverter/CSharp/CommonConversions.cs index bab4b4464..76e6c24f2 100644 --- a/CodeConverter/CSharp/CommonConversions.cs +++ b/CodeConverter/CSharp/CommonConversions.cs @@ -203,12 +203,10 @@ private async Task WithHoistedNodesInMethodAsync(VBSyntax.Modi private static string GetInitializerMethodName(ExpressionSyntax initializer, string fieldName) { - var invokedName = initializer is CSSyntax.InvocationExpressionSyntax { Expression: var invoked } ? invoked switch { - CSSyntax.SimpleNameSyntax name => name, - CSSyntax.MemberAccessExpressionSyntax memberAccess => memberAccess.Name, - _ => null - } : null; - return invokedName is null ? "init" + fieldName.UppercaseFirstLetter() : $"{invokedName.Identifier.ValueText}_{fieldName}"; + // Names are qualified by the time they're converted, e.g. Me.Foo(...) + return initializer is CSSyntax.InvocationExpressionSyntax { Expression: CSSyntax.MemberAccessExpressionSyntax { Name: var invokedName } } + ? $"{invokedName.Identifier.ValueText}_{fieldName}" + : "init" + fieldName.UppercaseFirstLetter(); } /// From c34406ecf3bc7888ed95ace9b906cf25efc3cf42 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 02:00:05 +0000 Subject: [PATCH 08/12] Emit each field initializer method once, including for WithEvents A field declaration's hoisted methods were emitted for every variable group of every declarator, so `A As Integer = Inc(C), B As Integer = Inc(C)` or `Arr(Inc(C)), Other As Integer` declared a method twice (CS0111/CS0121). A WithEvents field never emitted the method its constructor assignment calls (CS0103). Take the functions from the field scope as they're emitted, and emit them for WithEvents fields too. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FEr8FVAmHV9bscZ82CChg --- .../CSharp/DeclarationNodeVisitor.cs | 4 +- CodeConverter/CSharp/PerScopeState.cs | 10 +++++ .../VBToCS/ByRefWriteBackPlacementTests.vb | 37 +++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/CodeConverter/CSharp/DeclarationNodeVisitor.cs b/CodeConverter/CSharp/DeclarationNodeVisitor.cs index ae3660a4d..032bf7f5f 100644 --- a/CodeConverter/CSharp/DeclarationNodeVisitor.cs +++ b/CodeConverter/CSharp/DeclarationNodeVisitor.cs @@ -548,11 +548,11 @@ private IEnumerable CreateMemberDeclarations(IReadOnlyC } } + foreach (var method in CreateExtraMethodMembers()) yield return method; if (isWithEvents) { var fieldDecls = CreateWithEventsMembers(thisFieldModifiers, attributes, variablesDecl.Decl); foreach (var f in fieldDecls) yield return f; } else { - foreach (var method in CreateExtraMethodMembers()) yield return method; yield return CS.SyntaxFactory.FieldDeclaration(CS.SyntaxFactory.List(attributes), thisFieldModifiers, variablesDecl.Decl); } } @@ -580,7 +580,7 @@ private IEnumerable CreateWithEventsMembers(SyntaxToken private IReadOnlyCollection CreateExtraMethodMembers() { - var methodsInfos = AdditionalLocals.GetParameterlessFunctions(); + var methodsInfos = AdditionalLocals.TakeParameterlessFunctions(); // In theory this should use a unique name like in MethodBodyVisitor // Unfortunately we can't change the name for these since the file containing the constructor of a partial class tries to call the method with exactly this name. diff --git a/CodeConverter/CSharp/PerScopeState.cs b/CodeConverter/CSharp/PerScopeState.cs index eefdfdcb8..d0273911e 100644 --- a/CodeConverter/CSharp/PerScopeState.cs +++ b/CodeConverter/CSharp/PerScopeState.cs @@ -134,6 +134,16 @@ public IReadOnlyCollection GetParameterlessFunctions() return _hoistedNodesPerScope.Peek().OfType().ToArray(); } + /// + /// For a scope whose functions are emitted as they're found, e.g. one per variable of a field declaration, so each is emitted once. + /// + public IReadOnlyCollection TakeParameterlessFunctions() + { + var functions = GetParameterlessFunctions(); + _hoistedNodesPerScope.Peek().HoistedNodes.RemoveAll(n => n is HoistedFunction); + return functions; + } + public IReadOnlyCollection GetFields() { return _hoistedNodesPerScope.Peek().OfType().ToArray(); diff --git a/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb b/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb index c4eae72e8..eaedbf316 100644 --- a/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb +++ b/Tests/TestData/SelfVerifyingTests/VBToCS/ByRefWriteBackPlacementTests.vb @@ -331,6 +331,43 @@ Public Class ByRefWriteBackPlacementTests End Sub End Class + + Public Sub SeveralDeclaratorsInOneFieldDeclarationEachWriteBack() + Dim initialized As New SeveralDeclarators + Assert.Equal(1, initialized.A) + Assert.Equal(2, initialized.B) + Assert.Equal(4, initialized.Arr.Length) + Assert.Equal(3, initialized.Counter) + End Sub + + Private Class SeveralDeclarators + Public Property Counter As Integer + Public ReadOnly A As Integer = Increment(Counter), B As Integer = Increment(Counter) + Public Arr(Increment(Counter)), Other As Integer + End Class + + + Public Sub WithEventsFieldInitializerWritesBack() + Dim initialized As New WithEventsFieldInitializer + Assert.Equal(1, initialized.Counter) + Assert.Equal(1, initialized.Source.Received) + End Sub + + Private Class WithEventsFieldInitializer + Public Property Counter As Integer + Public WithEvents Source As New EventSource(Counter) + End Class + + Private Class EventSource + Public Event Changed() + Public ReadOnly Received As Integer + + Sub New(ByRef value As Integer) + value += 1 + Received = value + End Sub + End Class + Private Shared Function Increment(ByRef value As Integer) As Integer value += 1 Return value From e360affbbdf3cf8b78fd4e08846510d950a9c4b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 02:00:17 +0000 Subject: [PATCH 09/12] Document remaining ByRef write-back gaps found in review Reverse copy-back order of a call's arguments, and ReadOnly auto-properties written back in constructors and initializers. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FEr8FVAmHV9bscZ82CChg --- CodeConverter/CSharp/README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CodeConverter/CSharp/README.md b/CodeConverter/CSharp/README.md index aef66ce5f..47b13fccc 100644 --- a/CodeConverter/CSharp/README.md +++ b/CodeConverter/CSharp/README.md @@ -25,4 +25,8 @@ Sources: [passing arguments by value and by reference](https://learn.microsoft.c | Followed by other evaluation, or in a condition or block header (`If`, `ElseIf`, loops, `Select Case`, `Case`, `For`, `For Each`, `Using`, `SyncLock`, `With`, `Catch When`) | a local function that declares, calls and writes back, called in place | `NameExpressionNodeVisitor.RequiresLocalFunction`, `HoistAndCallLocalFunctionAsync` | | A field initializer | a method that declares, writes back and returns, called from the field (static) or the constructor (instance) | `CommonConversions.WithHoistedNodesInMethodAsync`, `PerScopeState.CreateReturnBlockAsync` | -Known gaps: arguments of `New` (object creation) in a block header still write back after the block, and a non-constant `For ... Step` is re-evaluated each iteration. +Known gaps: +* Arguments of `New` (object creation) in a block header still write back after the block. +* A non-constant `For ... Step` is re-evaluated each iteration (not ByRef-specific). +* VB copies back a call's arguments in reverse order (`copyBackArray.ReverseContents()` in `LocalRewriter_Call.vb`); the converter writes back in argument order. Only observable when two arguments write back to aliasing targets, e.g. `Two(p.Value, p.Value)`. A fix needs per-call grouping in `ArgumentConverter`, since nested calls write back before the outer call. +* A `ReadOnly` auto-property is assignable in its type's constructor and initializers, so VB writes it back there, but `GetRefConversionForExpression` treats every read-only property as not writable. A fix also needs the C# write-back target to be assignable from the field initializer method. From d7f02d1f843194a0b87b474f415c4dd8156ed385 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 03:15:21 +0000 Subject: [PATCH 10/12] Drop dead renaming of hoisted initializer methods Every caller now references these methods by their final name, so replacing placeholder ids in the constructor initializers never changed anything. Pure refactor, no output changes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FEr8FVAmHV9bscZ82CChg --- CodeConverter/CSharp/DeclarationNodeVisitor.cs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/CodeConverter/CSharp/DeclarationNodeVisitor.cs b/CodeConverter/CSharp/DeclarationNodeVisitor.cs index 032bf7f5f..7a35d2ef9 100644 --- a/CodeConverter/CSharp/DeclarationNodeVisitor.cs +++ b/CodeConverter/CSharp/DeclarationNodeVisitor.cs @@ -580,19 +580,9 @@ private IEnumerable CreateWithEventsMembers(SyntaxToken private IReadOnlyCollection CreateExtraMethodMembers() { - var methodsInfos = AdditionalLocals.TakeParameterlessFunctions(); - - // In theory this should use a unique name like in MethodBodyVisitor - // Unfortunately we can't change the name for these since the file containing the constructor of a partial class tries to call the method with exactly this name. - // See GetInitialValueFunctionName - var newMethodNames = methodsInfos.ToDictionary(l => l.Id, l => l.Prefix); - for (int i = 0; i < _typeContext.Initializers.AdditionalInstanceInitializers.Count; i++) { - var (a, b, initializer, _) = _typeContext.Initializers.AdditionalInstanceInitializers[i]; - _typeContext.Initializers.AdditionalInstanceInitializers[i] = new Assignment(a, b, PerScopeState.ReplaceNames(initializer, newMethodNames)); - } - - return methodsInfos - .Select(al => al.AsInstanceMethod(newMethodNames[al.Id])) + // Callers reference these by their final name, so it's the prefix rather than a placeholder + return AdditionalLocals.TakeParameterlessFunctions() + .Select(al => al.AsInstanceMethod(al.Prefix)) .ToArray(); } From 5043b0fb9b21d1dfa0c9663907730bcd91262642 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 03:18:36 +0000 Subject: [PATCH 11/12] Give field initializer methods names unused by other members A generated method like initW() or Inc_v() clashed with a user member of the same name (CS0111/CS0102/CS0121). Use the unique-name check already used for hoisted locals. Partial-class initialX() functions keep their fixed name, since another part calls them by it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FEr8FVAmHV9bscZ82CChg --- CodeConverter/CSharp/CommonConversions.cs | 3 +- Tests/CSharp/ExpressionTests/ByRefTests.cs | 59 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/CodeConverter/CSharp/CommonConversions.cs b/CodeConverter/CSharp/CommonConversions.cs index 76e6c24f2..dd19cdb98 100644 --- a/CodeConverter/CSharp/CommonConversions.cs +++ b/CodeConverter/CSharp/CommonConversions.cs @@ -196,7 +196,8 @@ private async Task WithHoistedNodesInMethodAsync(VBSyntax.Modi if (!_typeContext.PerScopeState.HasHoistedNodes) return initializer; var body = await _typeContext.PerScopeState.CreateReturnBlockAsync(vbName, initializer, type, SemanticModel); - var methodName = GetInitializerMethodName(initializer, ConvertIdentifier(vbName.Identifier).ValueText); + var baseMethodName = GetInitializerMethodName(initializer, ConvertIdentifier(vbName.Identifier).ValueText); + var methodName = NameGenerator.CS.GetUniqueVariableNameInScope(SemanticModel, new HashSet(), vbName, baseMethodName); _typeContext.PerScopeState.HoistToParent(new HoistedFunction(methodName, type, body, SyntaxFactory.ParameterList(), isStatic)); return SyntaxFactory.InvocationExpression(ValidSyntaxFactory.IdentifierName(methodName)); } diff --git a/Tests/CSharp/ExpressionTests/ByRefTests.cs b/Tests/CSharp/ExpressionTests/ByRefTests.cs index 24c6569b3..e96590161 100644 --- a/Tests/CSharp/ExpressionTests/ByRefTests.cs +++ b/Tests/CSharp/ExpressionTests/ByRefTests.cs @@ -1306,6 +1306,65 @@ public static int Inc(ref int v) v += 1; return v; } +}"); + } + + [Fact] + public async Task FieldInitializerMethodNamesAvoidExistingMembersAsync() + { + await TestConversionVisualBasicToCSharpAsync(@"Public Class C + Public Property P As Integer + Private w As Integer = Inc(P) + 1 + Private v As Integer = Inc(P) + Private Inc_v As Integer + + Function initW() As Integer + Return 0 + End Function + + Shared Function Inc(ByRef x As Integer) As Integer + x += 1 + Return x + End Function +End Class", @" +public partial class C +{ + public int P { get; set; } + + public C() + { + w = initW1(); + v = Inc_v1(); + } + int initW1() + { + int localInc() { int argx = P; var ret = Inc(ref argx); P = argx; return ret; } + + return localInc() + 1; + } + + private int w; + int Inc_v1() + { + int argx = P; + int ret = Inc(ref argx); + P = argx; + return ret; + } + + private int v; + private int Inc_v; + + public int initW() + { + return 0; + } + + public static int Inc(ref int x) + { + x += 1; + return x; + } }"); } } \ No newline at end of file From 9031b61c13e642030692b5855605a6c539bfdb98 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Nov 2026 03:18:46 +0000 Subject: [PATCH 12/12] Document further pre-existing ByRef gaps found in review Several declarators in one Dim, lambda parameters in ByRef arguments, auto-property initializers, and partial-class initializer order. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019FEr8FVAmHV9bscZ82CChg --- CodeConverter/CSharp/README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CodeConverter/CSharp/README.md b/CodeConverter/CSharp/README.md index 47b13fccc..d7b22694e 100644 --- a/CodeConverter/CSharp/README.md +++ b/CodeConverter/CSharp/README.md @@ -29,4 +29,8 @@ Known gaps: * Arguments of `New` (object creation) in a block header still write back after the block. * A non-constant `For ... Step` is re-evaluated each iteration (not ByRef-specific). * VB copies back a call's arguments in reverse order (`copyBackArray.ReverseContents()` in `LocalRewriter_Call.vb`); the converter writes back in argument order. Only observable when two arguments write back to aliasing targets, e.g. `Two(p.Value, p.Value)`. A fix needs per-call grouping in `ArgumentConverter`, since nested calls write back before the outer call. +* `Dim a = Inc(P), b = Inc(P)` writes back after all declarators, so `b` reads a stale `P`. +* A lambda parameter inside a ByRef argument (`Function(x) Inc(x.P)`) hoists the local function outside the lambda, where `x` isn't in scope (CS0103). +* An auto-property initializer (`Public Property X As Integer = Inc(P)`) leaves a `ph…` placeholder (CS0103): it doesn't go through the field initializer path. +* Partial-class instance initializers run in part order rather than declaration order. * A `ReadOnly` auto-property is assignable in its type's constructor and initializers, so VB writes it back there, but `GetRefConversionForExpression` treats every read-only property as not writable. A fix also needs the C# write-back target to be assignable from the field initializer method.