Skip to content

Commit

Permalink
Use immutable collections for bound nodes (#108)
Browse files Browse the repository at this point in the history
  • Loading branch information
ChrisKXu authored Aug 10, 2024
1 parent ccc91c3 commit feeae4b
Show file tree
Hide file tree
Showing 15 changed files with 74 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ static void GenerateBoundNodeFactoryMethods(SourceProductionContext context, Bou
using System;
using System.CodeDom.Compiler;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Reflection;
using Todl.Compiler.CodeAnalysis.Symbols;
using Todl.Compiler.CodeAnalysis.Syntax;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public void TestBindBlockStatement()
var boundBlockStatement = TestUtils.BindStatement<BoundBlockStatement>(input);

boundBlockStatement.Should().NotBeNull();
boundBlockStatement.Statements.Count.Should().Be(2);
boundBlockStatement.Statements.Should().HaveCount(2);
boundBlockStatement.Scope.LookupVariable("a").Type.SpecialType.Should().Be(SpecialType.ClrInt32);
boundBlockStatement.Scope.LookupVariable("b").Type.SpecialType.Should().Be(SpecialType.ClrInt32);

Expand All @@ -45,7 +45,7 @@ public void TestBindVariableDeclarationStatementBasic()
var boundBlockStatement = TestUtils.BindStatement<BoundBlockStatement>(input);

boundBlockStatement.Should().NotBeNull();
boundBlockStatement.Statements.Count.Should().Be(2);
boundBlockStatement.Statements.Should().HaveCount(2);
boundBlockStatement.Scope.LookupVariable("a").Type.SpecialType.Should().Be(SpecialType.ClrInt32);
boundBlockStatement.Scope.LookupVariable("b").Type.SpecialType.Should().Be(SpecialType.ClrInt32);
}
Expand All @@ -68,7 +68,7 @@ public void TestBindVariableDeclarationStatementWithNestedScope()
var boundBlockStatement = TestUtils.BindStatement<BoundBlockStatement>(input);

boundBlockStatement.Should().NotBeNull();
boundBlockStatement.Statements.Count.Should().Be(4);
boundBlockStatement.Statements.Should().HaveCount(4);

var scope = boundBlockStatement.Scope;
var childScope = (boundBlockStatement.Statements[2] as BoundBlockStatement).Scope;
Expand Down Expand Up @@ -102,7 +102,7 @@ public void TestBoundObjectCreationExpressionWithOneArgument(string inputText)
var exceptionType = TestDefaults.DefaultClrTypeCache.Resolve(typeof(Exception).FullName);
boundObjectCreationExpression.ResultType.Should().Be(exceptionType);
boundObjectCreationExpression.ConstructorInfo.Should().NotBeNull();
boundObjectCreationExpression.BoundArguments.Count.Should().Be(1);
boundObjectCreationExpression.BoundArguments.Should().HaveCount(1);

var message = boundObjectCreationExpression.BoundArguments[0].As<BoundConstant>();
message.Value.Should().Be("exception message");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public void TestBindClrFunctionCallExpressionWithOnePositionalArgument()
boundFunctionCallExpression.ResultType.SpecialType.Should().Be(SpecialType.ClrInt32);
boundFunctionCallExpression.MethodInfo.Name.Should().Be("Abs");
boundFunctionCallExpression.IsStatic.Should().Be(true);
boundFunctionCallExpression.BoundArguments.Count.Should().Be(1);
boundFunctionCallExpression.BoundArguments.Should().HaveCount(1);

var argument = boundFunctionCallExpression.BoundArguments[0].As<BoundUnaryExpression>();
argument.Operator.BoundUnaryOperatorKind.Should().Be(BoundUnaryOperatorKind.UnaryMinus | BoundUnaryOperatorKind.Int);
Expand All @@ -41,7 +41,7 @@ public void TestBindClrFunctionCallExpressionWithOneNamedArgument()
boundFunctionCallExpression.ResultType.SpecialType.Should().Be(SpecialType.ClrString);
boundFunctionCallExpression.MethodInfo.Name.Should().Be("ToString");
boundFunctionCallExpression.IsStatic.Should().Be(false);
boundFunctionCallExpression.BoundArguments.Count.Should().Be(1);
boundFunctionCallExpression.BoundArguments.Should().HaveCount(1);

var argument = boundFunctionCallExpression.BoundArguments[0].As<BoundConstant>();
argument.Value.Should().Be("G");
Expand All @@ -57,7 +57,7 @@ public void TestBindClrFunctionCallExpressionWithMultiplePositionalArguments()
boundFunctionCallExpression.IsStatic.Should().Be(false);

var boundArguments = boundFunctionCallExpression.BoundArguments;
boundArguments.Count.Should().Be(3);
boundArguments.Should().HaveCount(3);
boundArguments[0].As<BoundConstant>().Value.Should().Be("ab");
boundArguments[1].As<BoundConstant>().Value.Should().Be(1);
boundArguments[2].As<BoundConstant>().Value.Should().Be(2);
Expand All @@ -75,7 +75,7 @@ public void TestBindClrFunctionCallExpressionWithMultipleNamedArguments(string i
boundFunctionCallExpression.IsStatic.Should().Be(false);

var boundArguments = boundFunctionCallExpression.BoundArguments;
boundArguments.Count.Should().Be(2);
boundArguments.Should().HaveCount(2);
boundArguments[0].As<BoundConstant>().Value.Should().Be(1);
boundArguments[1].As<BoundConstant>().Value.Should().Be(2);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public void TestBindFunctionDeclarationMemberWithoutParametersOrBody(string inpu
{
var function = TestUtils.BindMember<BoundFunctionMember>(inputText);

function.Body.Statements.Count.Should().Be(expectedStatementsCount);
function.Body.Statements.Should().HaveCount(expectedStatementsCount);
function.ReturnType.Name.Should().Be(expectedReturnType.FullName);
function.ReturnType.IsArray.Should().Be(expectedReturnType.IsArray);
function.FunctionScope.BoundScopeKind.Should().Be(BoundScopeKind.Function);
Expand All @@ -33,7 +33,7 @@ public void TestBindFunctionDeclarationMemberWithBody()
var function = TestUtils.BindMember<BoundFunctionMember>(
inputText: "void Main() { const a = 30; System.Threading.Thread.Sleep(a); }");

function.Body.Statements.Count.Should().Be(3);
function.Body.Statements.Should().HaveCount(3);

var a = function.Body.Statements[0].As<BoundVariableDeclarationStatement>().Variable;
a.Name.Should().Be("a");
Expand All @@ -56,7 +56,7 @@ public void TestBindFunctionDeclarationMemberWithParameters()
a.Name.Should().Be("a");
a.Type.SpecialType.Should().Be(SpecialType.ClrInt32);

function.Body.Statements.Count.Should().Be(2);
function.Body.Statements.Should().HaveCount(2);
function.Body.Statements[0].As<BoundExpressionStatement>().Expression.As<BoundClrFunctionCallExpression>().Should().NotBeNull();

function.ReturnType.SpecialType.Should().Be(SpecialType.ClrVoid);
Expand Down Expand Up @@ -100,7 +100,7 @@ public void TestBindFunctionDeclarationMemberWithReturnStatement(string inputTex
var function = TestUtils.BindMember<BoundFunctionMember>(inputText);

var targetType = TestDefaults.DefaultClrTypeCache.Resolve(expectedReturnType.FullName);
function.Body.Statements.Count.Should().Be(1);
function.Body.Statements.Should().HaveCount(1);
function.ReturnType.Should().Be(targetType);
function.GetDiagnostics().Should().BeEmpty();

Expand All @@ -121,7 +121,7 @@ public void TestBindFunctionDeclarationMemberWithMismatchedReturnStatement(strin
var resolvedExpectedType = TestDefaults.DefaultClrTypeCache.Resolve(expectedReturnType.FullName);
var resolvedActualType = TestDefaults.DefaultClrTypeCache.Resolve(actualReturnType.FullName);

function.Body.Statements.Count.Should().Be(1);
function.Body.Statements.Should().HaveCount(1);
function.ReturnType.Should().Be(resolvedExpectedType);
function.GetDiagnostics().Should().NotBeEmpty();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public void BoundLoopStatementsCanHaveBody(string inputText, bool negated, int e
var boundLoopStatement = TestUtils.BindStatement<BoundLoopStatement>(inputText);
boundLoopStatement.GetDiagnostics().Should().BeEmpty();
boundLoopStatement.ConditionNegated.Should().Be(negated);
boundLoopStatement.Body.As<BoundBlockStatement>().Statements.Count.Should().Be(expectedBodyStatementsCount);
boundLoopStatement.Body.As<BoundBlockStatement>().Statements.Should().HaveCount(expectedBodyStatementsCount);
boundLoopStatement.BoundLoopContext.Should().NotBeNull();
}

Expand Down
7 changes: 4 additions & 3 deletions src/Todl.Compiler/CodeAnalysis/Binding/BoundNodeVisitor.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Todl.Compiler.CodeAnalysis.Binding.BoundTree;

Expand All @@ -18,7 +19,7 @@ public virtual BoundTodlTypeDefinition VisitBoundTypeDefinition(BoundTodlTypeDef
return new BoundEntryPointTypeDefinition()
{
SyntaxNode = boundTodlTypeDefinition.SyntaxNode,
BoundMembers = boundMembers,
BoundMembers = boundMembers.ToImmutableArray(),
DiagnosticBuilder = boundTodlTypeDefinition.DiagnosticBuilder
};
}
Expand Down Expand Up @@ -175,7 +176,7 @@ protected virtual BoundStatement VisitBoundBlockStatement(BoundBlockStatement bo
}

var hasChange = false;
var statements = new List<BoundStatement>();
var statements = ImmutableArray.CreateBuilder<BoundStatement>();

foreach (var boundStatement in boundBlockStatement.Statements)
{
Expand All @@ -196,7 +197,7 @@ protected virtual BoundStatement VisitBoundBlockStatement(BoundBlockStatement bo
return BoundNodeFactory.CreateBoundBlockStatement(
syntaxNode: boundBlockStatement.SyntaxNode,
scope: boundBlockStatement.Scope,
statements: statements,
statements: statements.ToImmutable(),
diagnosticBuilder: boundBlockStatement.DiagnosticBuilder);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Todl.Compiler.CodeAnalysis.Syntax;

Expand All @@ -8,7 +8,7 @@ namespace Todl.Compiler.CodeAnalysis.Binding.BoundTree;
internal sealed class BoundBlockStatement : BoundStatement
{
public BoundScope Scope { get; internal init; }
public IReadOnlyList<BoundStatement> Statements { get; internal init; }
public ImmutableArray<BoundStatement> Statements { get; internal init; }

public override BoundNode Accept(BoundTreeVisitor visitor) => visitor.VisitBoundBlockStatement(this);
}
Expand All @@ -19,7 +19,7 @@ private BoundBlockStatement BindBlockStatementInScope(BlockStatement blockStatem
=> BoundNodeFactory.CreateBoundBlockStatement(
syntaxNode: blockStatement,
scope: Scope,
statements: blockStatement.InnerStatements.Select(statement => BindStatement(statement)).ToList());
statements: ImmutableArray.CreateRange(blockStatement.InnerStatements.Select(BindStatement)));

private BoundBlockStatement BindBlockStatement(BlockStatement blockStatement)
=> CreateBlockStatementBinder().BindBlockStatementInScope(blockStatement);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
Expand All @@ -14,7 +14,7 @@ internal sealed class BoundClrFunctionCallExpression : BoundExpression
{
public BoundExpression BoundBaseExpression { get; internal init; }
public MethodInfo MethodInfo { get; internal init; }
public IReadOnlyList<BoundExpression> BoundArguments { get; internal init; }
public ImmutableArray<BoundExpression> BoundArguments { get; internal init; }
public override TypeSymbol ResultType => BoundBaseExpression.SyntaxNode.SyntaxTree.ClrTypeCache.Resolve(MethodInfo.ReturnType);
public bool IsStatic => MethodInfo.IsStatic;

Expand Down Expand Up @@ -86,13 +86,16 @@ private BoundClrFunctionCallExpression BindFunctionCallWithNamedArgumentsInterna
ReportNoMatchingFunctionCandidate(diagnosticBuilder, functionCallExpression);
}

var boundArguments = candidate?.GetParameters().OrderBy(p => p.Position).Select(p => arguments[p.Name]);
var boundArguments = candidate
.GetParameters()
.OrderBy(p => p.Position)
.Select(p => arguments[p.Name]);

return BoundNodeFactory.CreateBoundClrFunctionCallExpression(
syntaxNode: functionCallExpression,
boundBaseExpression: boundBaseExpression,
methodInfo: candidate,
boundArguments: boundArguments.ToList(),
boundArguments: boundArguments.ToImmutableArray(),
diagnosticBuilder: diagnosticBuilder);
}

Expand Down Expand Up @@ -123,7 +126,7 @@ private BoundClrFunctionCallExpression BindFunctionCallWithPositionalArgumentsIn
syntaxNode: functionCallExpression,
boundBaseExpression: boundBaseExpression,
methodInfo: candidate,
boundArguments: boundArguments.ToList(),
boundArguments: boundArguments.ToImmutableArray(),
diagnosticBuilder: diagnosticBuilder);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Todl.Compiler.CodeAnalysis.Symbols;
using Todl.Compiler.CodeAnalysis.Syntax;
Expand Down Expand Up @@ -88,7 +89,7 @@ internal BoundEntryPointTypeDefinition BindEntryPointTypeDefinition(IEnumerable<
return new()
{
SyntaxNode = null,
BoundMembers = boundMembers.ToList(),
BoundMembers = boundMembers.ToImmutableArray(),
DiagnosticBuilder = diagnosticBuilder
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Linq;
using System.Collections.Immutable;
using System.Linq;
using Todl.Compiler.CodeAnalysis.Symbols;
using Todl.Compiler.CodeAnalysis.Syntax;
using Todl.Compiler.Diagnostics;
Expand Down Expand Up @@ -60,7 +61,7 @@ private BoundFunctionMember BindFunctionDeclarationMember(FunctionDeclarationMem
body = BoundNodeFactory.CreateBoundBlockStatement(
syntaxNode: body.SyntaxNode,
scope: body.Scope,
statements: body.Statements.Append(returnStatement).ToList());
statements: body.Statements.Append(returnStatement).ToImmutableArray());
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
Expand All @@ -13,7 +13,7 @@ namespace Todl.Compiler.CodeAnalysis.Binding.BoundTree;
internal sealed class BoundObjectCreationExpression : BoundExpression
{
public ConstructorInfo ConstructorInfo { get; internal init; }
public IReadOnlyList<BoundExpression> BoundArguments { get; internal init; }
public ImmutableArray<BoundExpression> BoundArguments { get; internal init; }
public override TypeSymbol ResultType
=> SyntaxNode.SyntaxTree.ClrTypeCache.Resolve(ConstructorInfo.DeclaringType);

Expand All @@ -29,7 +29,7 @@ private BoundObjectCreationExpression BindNewExpression(NewExpression newExpress
diagnosticBuilder.Add(boundTypeExpression);

// Treating no arguments as the same way of positional arguments
if (!newExpression.Arguments.Items.Any() || !newExpression.Arguments.Items[0].IsNamedArgument)
if (newExpression.Arguments.Items.IsEmpty || !newExpression.Arguments.Items[0].IsNamedArgument)
{
return BindNewExpressionWithPositionalArgumentsInternal(
diagnosticBuilder: diagnosticBuilder,
Expand Down Expand Up @@ -67,7 +67,7 @@ private BoundObjectCreationExpression BindNewExpressionWithPositionalArgumentsIn
{
SyntaxNode = newExpression,
ConstructorInfo = constructorInfo,
BoundArguments = boundArguments.ToList(),
BoundArguments = boundArguments.ToImmutableArray(),
DiagnosticBuilder = diagnosticBuilder
};
}
Expand Down Expand Up @@ -112,7 +112,7 @@ private BoundObjectCreationExpression BindNewExpressionWithNamedArgumentsInterna
{
SyntaxNode = newExpression,
ConstructorInfo = constructorInfo,
BoundArguments = boundArguments.ToList(),
BoundArguments = boundArguments.ToImmutableArray(),
DiagnosticBuilder = diagnosticBuilder
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Todl.Compiler.CodeAnalysis.Symbols;
using Todl.Compiler.CodeAnalysis.Syntax;
Expand All @@ -10,7 +10,7 @@ namespace Todl.Compiler.CodeAnalysis.Binding.BoundTree;
internal sealed class BoundTodlFunctionCallExpression : BoundExpression
{
public FunctionSymbol FunctionSymbol { get; internal set; }
public IReadOnlyDictionary<string, BoundExpression> BoundArguments { get; internal init; }
public ImmutableDictionary<string, BoundExpression> BoundArguments { get; internal init; }

public override TypeSymbol ResultType
=> FunctionSymbol?.ReturnType ?? default; // TODO: we may need something like TypeSymbol.InvalidType for this
Expand All @@ -24,13 +24,13 @@ private BoundTodlFunctionCallExpression BindTodlFunctionCallExpression(FunctionC
{
var diagnosticBuilder = new DiagnosticBag.Builder();
FunctionSymbol functionSymbol = null;
IReadOnlyDictionary<string, BoundExpression> boundArguments = null;
var boundArguments = ImmutableDictionary<string, BoundExpression>.Empty;

var arguments = functionCallExpression.Arguments.Items;

if (arguments.Any(a => a.IsNamedArgument))
{
boundArguments = arguments.ToDictionary(
boundArguments = arguments.ToImmutableDictionary(
argument => argument.Identifier?.Text.ToString(),
argument => BindExpression(argument.Expression));

Expand All @@ -49,10 +49,10 @@ private BoundTodlFunctionCallExpression BindTodlFunctionCallExpression(FunctionC

boundArguments = functionSymbol?.OrderedParameterNames
.Zip(positionalArguments)
.ToDictionary(t => t.First, t => t.Second);
.ToImmutableDictionary(t => t.First, t => t.Second);
}

if (functionSymbol == null || boundArguments == null)
if (functionSymbol == null)
{
ReportNoMatchingFunctionCandidate(diagnosticBuilder, functionCallExpression);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
using System.Collections.Generic;
using System.Linq;
using System.Collections.Immutable;

namespace Todl.Compiler.CodeAnalysis.Binding.BoundTree;

internal abstract class BoundTodlTypeDefinition : BoundNode
{
public string Name { get; internal init; }
public IReadOnlyList<BoundMember> BoundMembers { get; internal init; }
public ImmutableArray<BoundMember> BoundMembers { get; internal init; }
public virtual bool IsStatic => false;
public virtual bool IsGeneratedType => false;

Expand Down
Loading

0 comments on commit feeae4b

Please sign in to comment.