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

Partial (simplest possible case) fix for OnError GoTo #999

Merged
merged 8 commits into from
May 4, 2023
Merged

Partial (simplest possible case) fix for OnError GoTo #999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0d92a20
Pass through statements
GrahamTheCoder Mar 26, 2023
08f862a
Move try/catch logic all to the same spot - and reuse method body vis…
GrahamTheCoder Mar 26, 2023
41da135
Generate a horrible do loop with a switch statement for handlers
GrahamTheCoder Mar 26, 2023
f0d3d5d
Handle one very specific case
GrahamTheCoder May 3, 2023
88bb233
Handle nulls and remove comment from old plan
GrahamTheCoder May 3, 2023
606122e
Separate method
GrahamTheCoder May 3, 2023
f706f05
Neaten up when there's no fall through
GrahamTheCoder May 4, 2023
dea4703
Add to changelog
GrahamTheCoder May 4, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

### VB -> C#

* Handle one very simple case of OnError Goto [#999](https://github.com/icsharpcode/CodeConverter/pull/999)
* Fix StackOverflowException when converting nullable comparisons [#1007](https://github.com/icsharpcode/CodeConverter/pull/1007)
* Fixes for default-initialized loop variables [#1001](https://github.com/icsharpcode/CodeConverter/pull/1001)
* Convert DefineDebug and DefineTrace into DefineConstants [#1004](https://github.com/icsharpcode/CodeConverter/pull/1004)
Expand Down
22 changes: 11 additions & 11 deletions CodeConverter/CSharp/DeclarationNodeVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ internal class DeclarationNodeVisitor : VBasic.VisualBasicSyntaxVisitor<Task<CSh
private string _topAncestorNamespace;

private CommonConversions CommonConversions { get; }
private Func<VisualBasicSyntaxNode, bool, IdentifierNameSyntax, Task<VisualBasicSyntaxVisitor<Task<SyntaxList<StatementSyntax>>>>> _createMethodBodyVisitorAsync { get; }
private Func<VisualBasicSyntaxNode, IReadOnlyCollection<VBSyntax.StatementSyntax>, bool, IdentifierNameSyntax, Task<IReadOnlyCollection<StatementSyntax>>> _convertMethodBodyStatementsAsync { get; }

internal PerScopeState AdditionalLocals => _typeContext.PerScopeState;

Expand All @@ -57,13 +57,14 @@ public DeclarationNodeVisitor(Document document, Compilation compilation, Semant
CommonConversions = new CommonConversions(document, semanticModel, typeConversionAnalyzer, csSyntaxGenerator, compilation, csCompilation, _typeContext, _visualBasicEqualityComparison);
var expressionNodeVisitor = new ExpressionNodeVisitor(semanticModel, _visualBasicEqualityComparison, _typeContext, CommonConversions, _extraUsingDirectives, _xmlImportContext, nullableExpressionsConverter);
_triviaConvertingExpressionVisitor = expressionNodeVisitor.TriviaConvertingExpressionVisitor;
_createMethodBodyVisitorAsync = expressionNodeVisitor.CreateMethodBodyVisitorAsync;
_convertMethodBodyStatementsAsync = expressionNodeVisitor.ConvertMethodBodyStatementsAsync;
CommonConversions.TriviaConvertingExpressionVisitor = _triviaConvertingExpressionVisitor;
}

public async Task<VBasic.VisualBasicSyntaxVisitor<Task<SyntaxList<StatementSyntax>>>> CreateMethodBodyVisitorAsync(VBasic.VisualBasicSyntaxNode node, bool isIterator = false, IdentifierNameSyntax csReturnVariable = null)
private async Task<IReadOnlyCollection<StatementSyntax>> ConvertMethodBodyStatementsAsync(VisualBasicSyntaxNode node,
IReadOnlyCollection<Microsoft.CodeAnalysis.VisualBasic.Syntax.StatementSyntax> statements, bool isIterator = false, IdentifierNameSyntax csReturnVariable = null)
{
return await _createMethodBodyVisitorAsync(node, isIterator, csReturnVariable);
return await _convertMethodBodyStatementsAsync(node, statements, isIterator, csReturnVariable);
}

public override async Task<CSharpSyntaxNode> DefaultVisit(SyntaxNode node)
Expand Down Expand Up @@ -944,7 +945,7 @@ public override async Task<CSharpSyntaxNode> VisitAccessorBlock(VBSyntax.Accesso
var ancestoryPropertyBlock = node.GetAncestor<VBSyntax.PropertyBlockSyntax>();
var containingPropertyStmt = ancestoryPropertyBlock?.PropertyStatement;
var csReturnVariableOrNull = CommonConversions.GetRetVariableNameOrNull(node);
var convertedStatements = await ConvertStatementsAsync(node.Statements, await CreateMethodBodyVisitorAsync(node, isIterator));
var convertedStatements = SyntaxFactory.Block(await ConvertMethodBodyStatementsAsync(node, node.Statements, isIterator));
var body = WithImplicitReturnStatements(node, convertedStatements, csReturnVariableOrNull);
var attributes = await CommonConversions.ConvertAttributesAsync(node.AccessorStatement.AttributeLists);
var modifiers = CommonConversions.ConvertModifiers(node, node.AccessorStatement.Modifiers, TokenContext.Local);
Expand Down Expand Up @@ -1037,8 +1038,7 @@ public override async Task<CSharpSyntaxNode> VisitMethodBlock(VBSyntax.MethodBlo
return methodBlock;
}
var csReturnVariableOrNull = CommonConversions.GetRetVariableNameOrNull(node);
var visualBasicSyntaxVisitor = await CreateMethodBodyVisitorAsync(node, node.IsIterator(), csReturnVariableOrNull);
var convertedStatements = await ConvertStatementsAsync(node.Statements, visualBasicSyntaxVisitor);
var convertedStatements = SyntaxFactory.Block(await ConvertMethodBodyStatementsAsync(node, node.Statements, node.IsIterator(), csReturnVariableOrNull));

// Just class events - for property events, see other use of IsDesignerGeneratedTypeWithInitializeComponent
if (node.SubOrFunctionStatement.Identifier.Text == "InitializeComponent" && node.SubOrFunctionStatement.IsKind(VBasic.SyntaxKind.SubStatement) && declaredSymbol.ContainingType.GetDesignerGeneratedInitializeComponentOrNull(_vbCompilation) != null) {
Expand Down Expand Up @@ -1464,8 +1464,8 @@ public override async Task<CSharpSyntaxNode> VisitOperatorStatement(VBSyntax.Ope
var attributeList = SyntaxFactory.List(attributes);
var returnType = await (node.AsClause?.Type).AcceptAsync<TypeSyntax>(_triviaConvertingExpressionVisitor) ?? SyntaxFactory.PredefinedType(SyntaxFactory.Token(CSSyntaxKind.VoidKeyword));
var parameterList = await node.ParameterList.AcceptAsync<ParameterListSyntax>(_triviaConvertingExpressionVisitor);
var methodBodyVisitor = await CreateMethodBodyVisitorAsync(node);
var body = SyntaxFactory.Block(await containingBlock.Statements.SelectManyAsync(async s => (IEnumerable<StatementSyntax>) await s.Accept(methodBodyVisitor)));
var methodBodyVisitor = await ConvertMethodBodyStatementsAsync(node, containingBlock.Statements);
var body = SyntaxFactory.Block(methodBodyVisitor);
var modifiers = CommonConversions.ConvertModifiers(node, node.Modifiers, GetMemberContext(node));

var conversionModifiers = modifiers.Where(CommonConversions.IsConversionOperator).ToList();
Expand Down Expand Up @@ -1505,14 +1505,14 @@ public override async Task<CSharpSyntaxNode> VisitConstructorBlock(VBSyntax.Cons
ctorCall = null;
}

var methodBodyVisitor = await CreateMethodBodyVisitorAsync(node);
var convertedBodyStatements = await ConvertMethodBodyStatementsAsync(node, statements.ToArray());
return SyntaxFactory.ConstructorDeclaration(
SyntaxFactory.List(attributes),
modifiers,
CommonConversions.ConvertIdentifier(node.GetAncestor<VBSyntax.TypeBlockSyntax>().BlockStatement.Identifier).WithoutSourceMapping(), //TODO Use semantic model for this name
await block.ParameterList.AcceptAsync<ParameterListSyntax>(_triviaConvertingExpressionVisitor),
ctorCall,
SyntaxFactory.Block(await statements.SelectManyAsync(async s => (IEnumerable<StatementSyntax>) await s.Accept(methodBodyVisitor)))
SyntaxFactory.Block(convertedBodyStatements)
);
}

Expand Down
99 changes: 50 additions & 49 deletions CodeConverter/CSharp/ExpressionNodeVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,46 +255,6 @@ public override async Task<CSharpSyntaxNode> VisitAwaitExpression(VBasic.Syntax.
return SyntaxFactory.AwaitExpression(await node.Expression.AcceptAsync<ExpressionSyntax>(TriviaConvertingExpressionVisitor));
}

public override async Task<CSharpSyntaxNode> VisitCatchBlock(VBasic.Syntax.CatchBlockSyntax node)
{
var stmt = node.CatchStatement;
CatchDeclarationSyntax catcher = null;
if (stmt.AsClause != null) {
catcher = SyntaxFactory.CatchDeclaration(
ConvertTypeSyntax(stmt.AsClause.Type),
ConvertIdentifier(stmt.IdentifierName.Identifier)
);
}

var filter = await stmt.WhenClause.AcceptAsync<CatchFilterClauseSyntax>(TriviaConvertingExpressionVisitor);
var methodBodyVisitor = await CreateMethodBodyVisitorAsync(node); //Probably should actually be using the existing method body visitor in order to get variable name generation correct
var stmts = await node.Statements.SelectManyAsync(async s => (IEnumerable<StatementSyntax>) await s.Accept(methodBodyVisitor));
return SyntaxFactory.CatchClause(
catcher,
filter,
SyntaxFactory.Block(stmts)
);
}

private TypeSyntax ConvertTypeSyntax(VBSyntax.TypeSyntax vbType)
{
if (_semanticModel.GetSymbolInfo(vbType).Symbol is ITypeSymbol typeSymbol)
return CommonConversions.GetTypeSyntax(typeSymbol);
return SyntaxFactory.ParseTypeName(vbType.ToString());
}

public override async Task<CSharpSyntaxNode> VisitCatchFilterClause(VBasic.Syntax.CatchFilterClauseSyntax node)
{
return SyntaxFactory.CatchFilterClause(await node.Filter.AcceptAsync<ExpressionSyntax>(TriviaConvertingExpressionVisitor));
}

public override async Task<CSharpSyntaxNode> VisitFinallyBlock(VBasic.Syntax.FinallyBlockSyntax node)
{
var methodBodyVisitor = await CreateMethodBodyVisitorAsync(node); //Probably should actually be using the existing method body visitor in order to get variable name generation correct
var stmts = await node.Statements.SelectManyAsync(async s => (IEnumerable<StatementSyntax>) await s.Accept(methodBodyVisitor));
return SyntaxFactory.FinallyClause(SyntaxFactory.Block(stmts));
}

public override async Task<CSharpSyntaxNode> VisitCTypeExpression(VBasic.Syntax.CTypeExpressionSyntax node)
{
var csharpArg = await node.Expression.AcceptAsync<ExpressionSyntax>(TriviaConvertingExpressionVisitor);
Expand Down Expand Up @@ -1273,7 +1233,7 @@ public override async Task<CSharpSyntaxNode> VisitSingleLineLambdaExpression(VBa
{
IReadOnlyCollection<StatementSyntax> convertedStatements;
if (node.Body is VBasic.Syntax.StatementSyntax statement) {
convertedStatements = await statement.Accept(await CreateMethodBodyVisitorAsync(node));
convertedStatements = await ConvertMethodBodyStatementsAsync(statement, statement.Yield().ToArray());
} else {
var csNode = await node.Body.AcceptAsync<ExpressionSyntax>(TriviaConvertingExpressionVisitor);
convertedStatements = new[] { SyntaxFactory.ExpressionStatement(csNode)};
Expand All @@ -1284,12 +1244,59 @@ public override async Task<CSharpSyntaxNode> VisitSingleLineLambdaExpression(VBa

public override async Task<CSharpSyntaxNode> VisitMultiLineLambdaExpression(VBasic.Syntax.MultiLineLambdaExpressionSyntax node)
{
var methodBodyVisitor = await CreateMethodBodyVisitorAsync(node);
var body = await node.Statements.SelectManyAsync(async s => (IEnumerable<StatementSyntax>) await s.Accept(methodBodyVisitor));
var body = await ConvertMethodBodyStatementsAsync(node, node.Statements);
var param = await node.SubOrFunctionHeader.ParameterList.AcceptAsync<ParameterListSyntax>(TriviaConvertingExpressionVisitor);
return await _lambdaConverter.ConvertAsync(node, param, body.ToList());
}

public async Task<IReadOnlyCollection<StatementSyntax>> ConvertMethodBodyStatementsAsync(VBasic.VisualBasicSyntaxNode node, IReadOnlyCollection<VBSyntax.StatementSyntax> statements, bool isIterator = false, IdentifierNameSyntax csReturnVariable = null)
{

var innerMethodBodyVisitor = await MethodBodyExecutableStatementVisitor.CreateAsync(node, _semanticModel, TriviaConvertingExpressionVisitor, CommonConversions, _withBlockLhs, _extraUsingDirectives, _typeContext, isIterator, csReturnVariable);

return await GetWithConvertedGotosOrNull(statements) ?? await ConvertStatements(statements);

async Task<List<StatementSyntax>> ConvertStatements(IEnumerable<VBSyntax.StatementSyntax> readOnlyCollection)
{
return (await readOnlyCollection.SelectManyAsync(async s => (IEnumerable<StatementSyntax>)await s.Accept(innerMethodBodyVisitor.CommentConvertingVisitor))).ToList();
}

async Task<IReadOnlyCollection<StatementSyntax>> GetWithConvertedGotosOrNull(IReadOnlyCollection<Microsoft.CodeAnalysis.VisualBasic.Syntax.StatementSyntax> statements)
{
var onlyIdentifierLabel = statements.OnlyOrDefault(s => s.IsKind(VBasic.SyntaxKind.LabelStatement));
var onlyOnErrorGotoStatement = statements.OnlyOrDefault(s => s.IsKind(VBasic.SyntaxKind.OnErrorGoToLabelStatement));

// See https://learn.microsoft.com/en-us/dotnet/visual-basic/language-reference/statements/on-error-statement
if (onlyIdentifierLabel != null && onlyOnErrorGotoStatement != null) {
var statementsList = statements.ToList();
var onlyIdentifierLabelIndex = statementsList.IndexOf(onlyIdentifierLabel);
var onlyOnErrorGotoStatementIndex = statementsList.IndexOf(onlyOnErrorGotoStatement);

// Even this very simple case can generate compile errors if the error handling uses statements declared in the scope of the try block
// For now, the user will have to fix these manually, in future it'd be possible to hoist any used declarations out of the try block
if (onlyOnErrorGotoStatementIndex < onlyIdentifierLabelIndex) {
var beforeStatements = await ConvertStatements(statements.Take(onlyOnErrorGotoStatementIndex));
var tryBlockStatements = await ConvertStatements(statements.Take(onlyIdentifierLabelIndex).Skip(onlyOnErrorGotoStatementIndex + 1));
var tryBlock = SyntaxFactory.Block(tryBlockStatements);
var afterStatements = await ConvertStatements(statements.Skip(onlyIdentifierLabelIndex + 1));

var catchClauseSyntax = SyntaxFactory.CatchClause();

// Default to putting the statements after the catch block in case logic falls through, but if the last statement is a return, put them inside the catch block for neatness.
if (tryBlockStatements.LastOrDefault().IsKind(SyntaxKind.ReturnStatement)) {
catchClauseSyntax = catchClauseSyntax.WithBlock(SyntaxFactory.Block(afterStatements));
afterStatements = new List<StatementSyntax>();
}

var tryStatement = SyntaxFactory.TryStatement(SyntaxFactory.SingletonList(catchClauseSyntax)).WithBlock(tryBlock);
return beforeStatements.Append(tryStatement).Concat(afterStatements).ToList();
}
}

return null;
}
}

public override async Task<CSharpSyntaxNode> VisitParameterList(VBSyntax.ParameterListSyntax node)
{
var parameters = await node.Parameters.SelectAsync(async p => await p.AcceptAsync<ParameterSyntax>(TriviaConvertingExpressionVisitor));
Expand Down Expand Up @@ -1577,12 +1584,6 @@ public override async Task<CSharpSyntaxNode> VisitTypeArgumentList(VBasic.Syntax
return SyntaxFactory.TypeArgumentList(SyntaxFactory.SeparatedList(args));
}

public async Task<VBasic.VisualBasicSyntaxVisitor<Task<SyntaxList<StatementSyntax>>>> CreateMethodBodyVisitorAsync(VBasic.VisualBasicSyntaxNode node, bool isIterator = false, IdentifierNameSyntax csReturnVariable = null)
{
var methodBodyVisitor = await MethodBodyExecutableStatementVisitor.CreateAsync(node, _semanticModel, TriviaConvertingExpressionVisitor, CommonConversions, _withBlockLhs, _extraUsingDirectives, _typeContext, isIterator, csReturnVariable);
return methodBodyVisitor.CommentConvertingVisitor;
}

private async Task<CSharpSyntaxNode> ConvertCastExpressionAsync(VBSyntax.CastExpressionSyntax node,
ExpressionSyntax convertMethodOrNull = null, VBSyntax.TypeSyntax castToOrNull = null)
{
Expand Down
Loading