From 944203579ac2fb49c2fe0c2f3d2756041e4fa776 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 04:29:21 +0000 Subject: [PATCH] fix: truncate integer division and remainder in Python and JavaScript [patch] The AST means / and % on two integers the way the C family does: the quotient truncates toward zero and the remainder takes the dividend's sign. Python's and JavaScript's / are true division, and Python's % floors, so -7 / 2 came out as -3.5 and -7 % 2 as 1. When both operands are statically integers, Python now writes int(a / b) and a - b * int(a / b) (bound through a lambda when an operand is not a literal or a variable, so it is evaluated once), and JavaScript writes Math.trunc(a / b). Both generators now open the static-type scope Go already used, so parameters and typed locals are known to be integers. Fixes #114 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AXWSrf4nXKEZF5xVNTiVV5 --- Coder.Test/Languages/IntegerDivisionTests.cs | 184 +++++++++++++++++++ Coder/Languages/JavaScriptGenerator.cs | 27 +++ Coder/Languages/PythonGenerator.cs | 61 ++++++ Coder/Languages/StandardLanguageGenerator.cs | 58 +++++- 4 files changed, 329 insertions(+), 1 deletion(-) create mode 100644 Coder.Test/Languages/IntegerDivisionTests.cs diff --git a/Coder.Test/Languages/IntegerDivisionTests.cs b/Coder.Test/Languages/IntegerDivisionTests.cs new file mode 100644 index 0000000..00d29df --- /dev/null +++ b/Coder.Test/Languages/IntegerDivisionTests.cs @@ -0,0 +1,184 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Coder.Test.Languages; + +using System.Globalization; +using System.Text; +using ktsu.Coder.Ast; +using ktsu.Coder.Languages; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests that integer division and remainder mean the same thing in Python and JavaScript as in the +/// C family. +/// +/// +/// The AST means -7 / 2 on two integers as truncating division, which is -3 in C, C++, +/// C#, Go and Rust. Python's and JavaScript's / are true division and give -3.5, and +/// Python's % floors, giving 1 for -7 % 2 where the C family gives -1. +/// The generated code is run rather than read, because the spelling of a correct quotient is a choice +/// and its value is not. +/// +[TestClass] +public class IntegerDivisionTests +{ + private static readonly (int Left, int Right)[] Operands = [(-7, 2), (7, -2), (-7, -2), (7, 2), (-6, 3), (0, -5)]; + + /// + /// The functions under test: integer parameters, integer literals, a typed local, and an operand + /// that is a call, which is the one a remainder must not evaluate twice. + /// + private static IEnumerable Functions() + { + foreach (BinaryOperator op in new[] { BinaryOperator.Divide, BinaryOperator.Modulo }) + { + string name = op == BinaryOperator.Divide ? "div" : "mod"; + + FunctionDeclaration parameters = new($"{name}_params") { ReturnType = new TypeReference("int") }; + parameters.Parameters.Add(new Parameter("a", "int")); + parameters.Parameters.Add(new Parameter("b", "int")); + parameters.Body.Add(new ReturnStatement(new BinaryExpression( + new VariableReference("a"), op, new VariableReference("b")))); + yield return parameters; + + FunctionDeclaration literals = new($"{name}_literals") { ReturnType = new TypeReference("int") }; + literals.Body.Add(new ReturnStatement(new BinaryExpression( + new LiteralExpression(-7), op, new LiteralExpression(2)))); + yield return literals; + + FunctionDeclaration local = new($"{name}_local") { ReturnType = new TypeReference("int") }; + local.Parameters.Add(new Parameter("a", "int")); + local.Parameters.Add(new Parameter("b", "int")); + local.Body.Add(new VariableDeclaration("q", "int", new VariableReference("a"))); + local.Body.Add(new ReturnStatement(new BinaryExpression( + new VariableReference("q"), op, new VariableReference("b")))); + yield return local; + + FunctionDeclaration call = new($"{name}_call") { ReturnType = new TypeReference("int") }; + call.Parameters.Add(new Parameter("a", "int")); + call.Parameters.Add(new Parameter("b", "int")); + CallExpression counted = new("counted") { ExpectedType = "int" }; + counted.Arguments.Add(new VariableReference("a")); + call.Body.Add(new ReturnStatement(new BinaryExpression(counted, op, new VariableReference("b")))); + yield return call; + } + } + + private static int Expected(string function, int left, int right) + { + bool divide = function.StartsWith("div", StringComparison.Ordinal); + (int a, int b) = function.EndsWith("_literals", StringComparison.Ordinal) ? (-7, 2) : (left, right); + return divide ? a / b : a % b; + } + + /// + /// Tests that Python computes the C family's quotient and remainder, and calls an operand once. + /// + [TestMethod] + public void Python_MatchesTheCFamily() + { + string? python = ToolchainHarness.FindOnPath("--version", "python3", "python"); + if (python is null) + { + Assert.Inconclusive("No Python interpreter on the path, so nothing was run."); + return; + } + + PythonGenerator generator = new(); + StringBuilder module = new(); + module.AppendLine("calls = 0"); + module.AppendLine("def counted(x):"); + module.AppendLine(" global calls"); + module.AppendLine(" calls += 1"); + module.AppendLine(" return x"); + foreach (FunctionDeclaration function in Functions()) + { + module.AppendLine(generator.Generate(function)); + } + + foreach (FunctionDeclaration function in Functions()) + { + foreach ((int left, int right) in Operands) + { + string arguments = function.Parameters.Count == 0 + ? string.Empty + : string.Create(CultureInfo.InvariantCulture, $"{left}, {right}"); + int expected = Expected(function.Name!, left, right); + module.AppendLine(CultureInfo.InvariantCulture, $"calls = 0"); + module.AppendLine(CultureInfo.InvariantCulture, $"r = {function.Name}({arguments})"); + module.AppendLine(CultureInfo.InvariantCulture, $"assert type(r) is int and r == {expected}, f\"{function.Name}({arguments}) = {{r!r}}, expected {expected}\""); + if (function.Name!.EndsWith("_call", StringComparison.Ordinal)) + { + module.AppendLine(CultureInfo.InvariantCulture, $"assert calls == 1, f\"{function.Name} called its operand {{calls}} times\""); + } + } + } + + Run(python, "check.py", module.ToString()); + } + + /// + /// Tests that JavaScript computes the C family's quotient and remainder. + /// + [TestMethod] + public void JavaScript_MatchesTheCFamily() + { + string? node = ToolchainHarness.FindOnPath("--version", "node"); + if (node is null) + { + Assert.Inconclusive("No JavaScript runtime on the path, so nothing was run."); + return; + } + + JavaScriptGenerator generator = new(); + StringBuilder module = new(); + module.AppendLine("function counted(x) { return x; }"); + foreach (FunctionDeclaration function in Functions()) + { + module.AppendLine(generator.Generate(function)); + } + + foreach (FunctionDeclaration function in Functions()) + { + foreach ((int left, int right) in Operands) + { + string arguments = function.Parameters.Count == 0 + ? string.Empty + : string.Create(CultureInfo.InvariantCulture, $"{left}, {right}"); + int expected = Expected(function.Name!, left, right); + + // Strict equality rather than Object.is: JavaScript's -0 is an integer zero to every + // arithmetic and comparison a consumer would make of it. + module.AppendLine(CultureInfo.InvariantCulture, $"{{ const r = {function.Name}({arguments}); if (r !== {expected}) {{ throw new Error(`{function.Name}({arguments}) = ${{r}}, expected {expected}`); }} }}"); + } + } + + Run(node, "check.js", module.ToString()); + } + + /// + /// Tests that a division the AST does not know to be on integers is left as true division. + /// + /// The generator to ask. + [TestMethod] + [DataRow("python")] + [DataRow("javascript")] + public void NonIntegerDivision_IsUnchanged(string language) + { + ILanguageGenerator generator = language == "python" ? new PythonGenerator() : new JavaScriptGenerator(); + + BinaryExpression doubles = new(new LiteralExpression(7.0), BinaryOperator.Divide, new LiteralExpression(2)); + BinaryExpression unknown = new(new VariableReference("a"), BinaryOperator.Divide, new VariableReference("b")); + + Assert.AreEqual("(7.0 / 2)", generator.Generate(doubles).Trim()); + Assert.AreEqual("(a / b)", generator.Generate(unknown).Trim()); + } + + private static void Run(string runtime, string fileName, string source) => + ToolchainHarness.InTemporaryDirectory(directory => + { + File.WriteAllText(Path.Combine(directory, fileName), source); + (int exitCode, string output) = ToolchainHarness.Run(runtime, fileName, directory); + Assert.AreEqual(0, exitCode, $"{output}{Environment.NewLine}{source}"); + }); +} diff --git a/Coder/Languages/JavaScriptGenerator.cs b/Coder/Languages/JavaScriptGenerator.cs index 02709a7..ba57f49 100644 --- a/Coder/Languages/JavaScriptGenerator.cs +++ b/Coder/Languages/JavaScriptGenerator.cs @@ -327,6 +327,7 @@ protected override void GenerateFunctionDeclaration(FunctionDeclaration funcDecl Ensure.NotNull(funcDecl); Ensure.NotNull(code); + using IDisposable types = OpenStaticTypeScope(funcDecl); WritePreamble(funcDecl, code, PreambleSite.Function); code.Write($"function {funcDecl.Name ?? "unnamedFunction"}("); @@ -455,6 +456,7 @@ private void GenerateField(VariableDeclaration field, CodeBlocker code) /// The writer to emit into. private void GenerateMethod(FunctionDeclaration method, TypeReference? baseType, CodeBlocker code) { + using IDisposable types = OpenStaticTypeScope(method); WritePreamble(method, code, PreambleSite.Method); if (method.Definition != FunctionDefinition.Provided) @@ -575,6 +577,7 @@ protected override void GenerateVariableDeclaration(VariableDeclaration varDecl, { Ensure.NotNull(varDecl); Ensure.NotNull(code); + RegisterStaticType(varDecl); // `const` needs an initializer, so an uninitialized constant has to be declared with `let`. string keyword = varDecl.IsConstant && varDecl.InitialValue is not null ? "const" : "let"; @@ -628,6 +631,30 @@ protected override void GenerateEntryPoint(EntryPoint entryPoint, CodeBlocker co : $"main({arguments});"); } + /// + /// + /// The AST means integer the way the C family does, truncating + /// toward zero, so -7 / 2 is -3. JavaScript has one number type and its / is + /// true division, giving -3.5, so when both operands are known to be integers the quotient + /// is truncated with Math.trunc — not Math.floor, which would give -4. + /// JavaScript's % already takes the sign of the dividend, the same as the C family's. + /// + protected override void GenerateBinary(BinaryExpression binary, CodeBlocker code) + { + Ensure.NotNull(binary); + Ensure.NotNull(code); + + if (binary.Operator != BinaryOperator.Divide || !IsIntegerArithmetic(binary)) + { + base.GenerateBinary(binary, code); + return; + } + + code.Write("Math.trunc("); + GenerateBinaryExpression(binary, code, "/"); + code.Write(")"); + } + /// /// Maps a binary operator to its JavaScript spelling. /// diff --git a/Coder/Languages/PythonGenerator.cs b/Coder/Languages/PythonGenerator.cs index a097a57..986f9df 100644 --- a/Coder/Languages/PythonGenerator.cs +++ b/Coder/Languages/PythonGenerator.cs @@ -421,6 +421,7 @@ protected override void GenerateFunctionDeclaration(FunctionDeclaration funcDecl Ensure.NotNull(funcDecl); Ensure.NotNull(code); + using IDisposable types = OpenStaticTypeScope(funcDecl); WritePreamble(funcDecl, code, PreambleSite.Function); // Function signature @@ -578,6 +579,7 @@ .. classDecl.Interfaces.Select(implemented => PythonBaseFromGenericType(implemen /// private void GenerateMethod(FunctionDeclaration method, ClassDeclaration declaration, CodeBlocker code) { + using IDisposable types = OpenStaticTypeScope(method); WritePreamble(method, code, PreambleSite.Method); if (method.Definition != FunctionDefinition.Provided) @@ -915,6 +917,7 @@ protected override void GenerateVariableDeclaration(VariableDeclaration varDecl, { Ensure.NotNull(varDecl); Ensure.NotNull(code); + RegisterStaticType(varDecl); code.Write(varDecl.Name); @@ -930,6 +933,64 @@ protected override void GenerateVariableDeclaration(VariableDeclaration varDecl, } } + /// + /// + /// The AST means integer and + /// the way the C family does: the quotient truncates toward zero and the remainder takes the sign + /// of the dividend, so -7 / 2 is -3 and -7 % 2 is -1. Python's / + /// is true division and gives -3.5, a float, and its // and % floor, giving + /// -4 and 1. So when both operands are known to be integers the quotient is truncated + /// with int(), and the remainder is what is left once the truncated quotient is taken away. + /// + /// The remainder names each operand twice. An operand that is a literal or a variable reads the + /// same both times; anything else is bound once, as the parameters of a lambda called where it + /// stands, so a call is not made twice. math.fmod would say it in one call, but it needs an + /// import and an expression can be generated outside any file that could carry one. + /// + /// + protected override void GenerateBinary(BinaryExpression binary, CodeBlocker code) + { + Ensure.NotNull(binary); + Ensure.NotNull(code); + + if (binary.Operator is not (BinaryOperator.Divide or BinaryOperator.Modulo) || !IsIntegerArithmetic(binary)) + { + base.GenerateBinary(binary, code); + return; + } + + if (binary.Operator == BinaryOperator.Divide) + { + code.Write("int("); + GenerateBinaryExpression(binary, code, "/"); + code.Write(")"); + return; + } + + if (IsReadTwiceSafely(binary.Left) && IsReadTwiceSafely(binary.Right)) + { + code.Write("("); + GenerateInternal(binary.Left, code); + code.Write(" - "); + GenerateInternal(binary.Right, code); + code.Write(" * int("); + GenerateBinaryExpression(binary, code, "/"); + code.Write("))"); + return; + } + + code.Write("(lambda a, b: a - b * int(a / b))("); + GenerateInternal(binary.Left, code); + code.Write(", "); + GenerateInternal(binary.Right, code); + code.Write(")"); + } + + private static bool IsReadTwiceSafely(Expression operand) => + operand is VariableReference + or LiteralExpression + or LiteralExpression; + /// /// Maps a binary operator to its Python spelling. /// diff --git a/Coder/Languages/StandardLanguageGenerator.cs b/Coder/Languages/StandardLanguageGenerator.cs index 3495750..3524a10 100644 --- a/Coder/Languages/StandardLanguageGenerator.cs +++ b/Coder/Languages/StandardLanguageGenerator.cs @@ -99,6 +99,42 @@ protected void PushStaticTypeScope(FunctionDeclaration function) /// protected void PopStaticTypeScope() => staticTypeScopes.Pop(); + /// + /// Starts a function's type scope and ends it when the returned guard is disposed. + /// + /// The function whose body is about to be generated. + /// The guard whose disposal ends the scope. + protected IDisposable OpenStaticTypeScope(FunctionDeclaration function) + { + PushStaticTypeScope(function); + return new StaticTypeScopeGuard(this); + } + + /// + /// Reads whether both operands of a binary expression are statically known to be integers. + /// + /// The expression to inspect. + /// True when the AST establishes an integer type for each operand. + /// + /// This is what separates -7 / 2, which the AST means as truncating integer division because + /// that is what it is in C, C++, C#, Go and Rust, from a division the AST cannot say anything about. + /// An operand whose type is unknown is not assumed to be an integer. + /// + protected bool IsIntegerArithmetic(BinaryExpression binary) + { + Ensure.NotNull(binary); + return IsIntegerType(StaticType(binary.Left)) && IsIntegerType(StaticType(binary.Right)); + } + + private static bool IsIntegerType(TypeReference? type) => + type is { IsArray: false, Indirection: TypeIndirection.None, TypeArguments.Count: 0 } + && IntegerTypeNames.Contains(type.Name); + + private static readonly HashSet IntegerTypeNames = new(StringComparer.Ordinal) + { + "int", "long", "short", "byte", "sbyte", "uint", "ulong", "ushort", + }; + /// /// Records a local's declared or inferred type for later references. /// @@ -187,6 +223,11 @@ or BinaryOperator.LogicalAnd private sealed record StaticTypeScope(Dictionary Types, TypeReference? ReturnType); + private sealed class StaticTypeScopeGuard(StandardLanguageGenerator generator) : IDisposable + { + public void Dispose() => generator.PopStaticTypeScope(); + } + /// /// Dispatches a node to the emitter for its shape. /// @@ -265,7 +306,7 @@ protected sealed override void GenerateInternal(AstNode node, CodeBlocker code) break; case BinaryExpression binaryExpr: - GenerateBinaryExpression(binaryExpr, code, GetOperatorSpelling(binaryExpr.Operator)); + GenerateBinary(binaryExpr, code); break; case UnaryExpression unaryExpr: @@ -893,6 +934,21 @@ private static bool HasSymbol(UnaryOperator op) => /// protected virtual string GetOperatorSpelling(BinaryOperator op) => GetBinaryOperator(op); + /// + /// Emits a binary expression in the target language. + /// + /// The expression to emit. + /// The writer to emit into. + /// + /// Defaults to the operands either side of the operator's spelling. A language whose operator + /// means something else for some operands, rather than being spelled differently, overrides this. + /// + protected virtual void GenerateBinary(BinaryExpression binary, CodeBlocker code) + { + Ensure.NotNull(binary); + GenerateBinaryExpression(binary, code, GetOperatorSpelling(binary.Operator)); + } + /// /// Spells a unary operator in the target language. ///