Skip to content

Commit

Permalink
(interpreter) Make operand type checks be compile-time errors (#329)
Browse files Browse the repository at this point in the history
  • Loading branch information
perlun authored May 11, 2022
1 parent 432a80f commit de3ff2e
Show file tree
Hide file tree
Showing 24 changed files with 573 additions and 489 deletions.
1 change: 1 addition & 0 deletions Perlang.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
<s:Boolean x:Key="/Default/UserDictionary/Words/=getenv/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=getpid/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=getppid/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=incoercible/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=libc/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=numerability/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Pascalize/@EntryIndexedValue">True</s:Boolean>
Expand Down
4 changes: 3 additions & 1 deletion release-notes/v0.2.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

### Changed
- Refactor number parsing to use *Literal types [[#308][308]]
- Make operand type checks be compile-time errors [[#329][329]]

### Fixed
- Fix broken support for certain negative integer literals [[#302][302]]
Expand All @@ -13,7 +14,7 @@
- Fix binary operator support for `int` + `long` [[#328][328]]

### Tests
- Greatly increased the test coverage in general. The number of tests increased from 537 tests in the previous release to 1337 tests in version 0.2.0.
- Greatly increased the test coverage in general. The number of tests increased from 537 tests in the previous release to 1334 tests in version 0.2.0.
- Improve coverage of binary operator type combinations [[#313][313]]
- Add test to ensure `BinaryoperatorData` covers all numeric type combinations [[#316][316]]
- Add full type coverage for `+` and `-` operators [[#317][317]]
Expand Down Expand Up @@ -45,3 +46,4 @@
[326]: https://github.com/perlang-org/perlang/pull/326
[327]: https://github.com/perlang-org/perlang/pull/327
[328]: https://github.com/perlang-org/perlang/pull/328
[329]: https://github.com/perlang-org/perlang/pull/329
2 changes: 0 additions & 2 deletions src/Perlang.Common/TypeReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ namespace Perlang
/// </summary>
public class TypeReference : ITypeReference
{
public static TypeReference Bool { get; } = new TypeReference(typeof(bool));

private Type? clrType;
public Token? TypeSpecifier { get; }

Expand Down
16 changes: 16 additions & 0 deletions src/Perlang.Interpreter/Extensions/TokenTypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,16 @@ public static class TokenTypeExtensions
/// <returns>The source-level representation of the given token type.</returns>
public static string ToSourceString(this TokenType tokenType)
{
// I have thought about making a unit test for this (to ensure all token types are properly handled here),
// but the problem is that we have token types like IDENTIFIER, STRING and so forth, which doesn't really
// represent a constant "source string" in that sense. If we ever decide to group tokens in "constant" and
// "variadic" groups or something, it could potentially be doable though. Even just "single-letter" and
// "two-letter" groupings would be useful to begin with.

return tokenType switch
{
TokenType.PLUS =>
"+",
TokenType.PLUS_EQUAL =>
"+=",
TokenType.MINUS =>
Expand All @@ -30,6 +38,14 @@ public static string ToSourceString(this TokenType tokenType)
"**",
TokenType.PERCENT =>
"%",
TokenType.LESS =>
"<",
TokenType.LESS_EQUAL =>
"<=",
TokenType.GREATER =>
">",
TokenType.GREATER_EQUAL =>
">=",
TokenType.LESS_LESS =>
"<<",
TokenType.GREATER_GREATER =>
Expand Down
13 changes: 13 additions & 0 deletions src/Perlang.Interpreter/InterpreterMessages.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using Perlang.Interpreter.Extensions;
using static Perlang.Utils;

namespace Perlang.Interpreter;

public static class InterpreterMessages
{
public static string UnsupportedOperandTypes(TokenType operatorType, object left, object right) =>
$"Unexpected runtime error: Unsupported {operatorType.ToSourceString()} operands specified: {StringifyType(left)} and {StringifyType(right)}";

public static string UnsupportedOperatorTypeInBinaryExpression(TokenType operatorType) =>
$"Internal error: Unsupported operator {operatorType.ToSourceString()} in binary expression.";
}
81 changes: 58 additions & 23 deletions src/Perlang.Interpreter/PerlangInterpreter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,8 @@ private static void CheckNumberOperands(Token @operator, object? left, object? r
return;
}

// TODO: We currently get this for `string + null`, which doesn't feel right. See
// TODO: https://github.com/perlang-org/perlang/issues/330 for more details.
throw new RuntimeError(@operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
}

Expand Down Expand Up @@ -1204,7 +1206,10 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
throw new RuntimeError(expr.Operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

case GREATER_EQUAL:
Expand Down Expand Up @@ -1262,7 +1267,10 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
throw new RuntimeError(expr.Operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

case LESS:
Expand Down Expand Up @@ -1320,7 +1328,10 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
throw new RuntimeError(expr.Operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

case LESS_EQUAL:
Expand Down Expand Up @@ -1378,7 +1389,10 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
throw new RuntimeError(expr.Operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

case BANG_EQUAL:
Expand Down Expand Up @@ -1454,7 +1468,10 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
throw new RuntimeError(expr.Operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

case PLUS:
Expand Down Expand Up @@ -1558,7 +1575,10 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
throw new RuntimeError(expr.Operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

case PLUS_EQUAL:
Expand Down Expand Up @@ -1624,7 +1644,10 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
throw new RuntimeError(expr.Operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

case SLASH:
Expand Down Expand Up @@ -1689,7 +1712,10 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
throw new RuntimeError(expr.Operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

case STAR:
Expand Down Expand Up @@ -1754,8 +1780,10 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
// TODO: "Operands must be numbers" isn't completely correct here.
throw new RuntimeError(expr.Operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

case STAR_STAR:
Expand Down Expand Up @@ -1802,9 +1830,10 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
// Should in practice never get here, since TypeResolver should catch it at compile-time. If it
// ever happens, it is to be considered an ICE.
throw new RuntimeError(expr.Operator, $"Internal compiler error: Unsupported ** operands specified: {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

case PERCENT:
Expand Down Expand Up @@ -1869,8 +1898,10 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
// TODO: "Operands must be numbers" isn't completely correct here.
throw new RuntimeError(expr.Operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

case LESS_LESS:
Expand Down Expand Up @@ -1899,10 +1930,10 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
// TODO: "Operands must be numbers" isn't completely correct here. It should be fine once we
// TODO: have gotten rid of these and made all such errors (compile-time) validation errors instead.
// TODO: That way, this will just be a "fallback", an escape hatch if you will.
throw new RuntimeError(expr.Operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

case GREATER_GREATER:
Expand Down Expand Up @@ -1931,13 +1962,17 @@ public VoidObject VisitWhileStmt(Stmt.While stmt)
}
else
{
// TODO: "Operands must be numbers" isn't completely correct here. The operands may very well
// _be_ valid numbers, but just not an accepted combination of number types.
throw new RuntimeError(expr.Operator, $"Operands must be numbers, not {StringifyType(left)} and {StringifyType(right)}");
// Should in practice never get here, since TypeResolver should catch these at compile-time. If
// it ever happens, it indicates an error somewhere.
string message = InterpreterMessages.UnsupportedOperandTypes(expr.Operator.Type, left, right);
throw new RuntimeError(expr.Operator, message);
}

default:
throw new RuntimeError(expr.Operator, $"Internal error: Unsupported operator {expr.Operator.Type} in binary expression.");
{
string message = InterpreterMessages.UnsupportedOperatorTypeInBinaryExpression(expr.Operator.Type);
throw new RuntimeError(expr.Operator, message);
}
}
}

Expand Down
12 changes: 12 additions & 0 deletions src/Perlang.Interpreter/Typing/Messages.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#nullable enable
using Perlang.Extensions;
using Perlang.Interpreter.Extensions;

namespace Perlang.Interpreter.Typing;

internal static class Messages
{
public static string UnsupportedOperandTypes(TokenType operatorType, ITypeReference leftTypeReference, ITypeReference rightTypeReference) =>
$"Unsupported {operatorType.ToSourceString()} operand types: " +
$"'{leftTypeReference.ClrType.ToTypeKeyword()}' and '{rightTypeReference.ClrType.ToTypeKeyword()}'";
}
Loading

0 comments on commit de3ff2e

Please sign in to comment.