diff --git a/lib/std/zig/c_translation.zig b/lib/std/zig/c_translation.zig index 999572d212..3c7749b66a 100644 --- a/lib/std/zig/c_translation.zig +++ b/lib/std/zig/c_translation.zig @@ -346,6 +346,17 @@ test "Flexible Array Type" { try testing.expectEqual(FlexibleArrayType(*const volatile Container, c_int), [*c]const volatile c_int); } +/// C `%` operator for signed integers +/// C standard states: "If the quotient a/b is representable, the expression (a/b)*b + a%b shall equal a" +/// The quotient is not representable if denominator is zero, or if numerator is the minimum integer for +/// the type and denominator is -1. C has undefined behavior for those two cases; this function has safety +/// checked undefined behavior +pub fn signedRemainder(numerator: anytype, denominator: anytype) @TypeOf(numerator, denominator) { + std.debug.assert(@typeInfo(@TypeOf(numerator, denominator)).Int.signedness == .signed); + if (denominator > 0) return @rem(numerator, denominator); + return numerator - @divTrunc(numerator, denominator) * denominator; +} + pub const Macros = struct { pub fn U_SUFFIX(comptime n: comptime_int) @TypeOf(promoteIntLiteral(c_uint, n, .decimal)) { return promoteIntLiteral(c_uint, n, .decimal); diff --git a/src/translate_c.zig b/src/translate_c.zig index 738de43e1b..5358154e5d 100644 --- a/src/translate_c.zig +++ b/src/translate_c.zig @@ -1620,10 +1620,10 @@ fn transBinaryOperator( }, .Rem => { if (cIsSignedInteger(qt)) { - // signed integer division uses @rem + // signed integer remainder uses std.zig.c_translation.signedRemainder const lhs = try transExpr(c, scope, stmt.getLHS(), .used); const rhs = try transExpr(c, scope, stmt.getRHS(), .used); - const rem = try Tag.rem.create(c.arena, .{ .lhs = lhs, .rhs = rhs }); + const rem = try Tag.signed_remainder.create(c.arena, .{ .lhs = lhs, .rhs = rhs }); return maybeSuppressResult(c, scope, result_used, rem); } }, @@ -3831,7 +3831,7 @@ fn transCreateCompoundAssign( if (requires_int_cast) rhs_node = try transCCast(c, scope, loc, lhs_qt, rhs_qt, rhs_node); const operands = .{ .lhs = lhs_node, .rhs = rhs_node }; const builtin = if (is_mod) - try Tag.rem.create(c.arena, operands) + try Tag.signed_remainder.create(c.arena, operands) else try Tag.div_trunc.create(c.arena, operands); @@ -3871,7 +3871,7 @@ fn transCreateCompoundAssign( if (requires_int_cast) rhs_node = try transCCast(c, scope, loc, lhs_qt, rhs_qt, rhs_node); const operands = .{ .lhs = ref_node, .rhs = rhs_node }; const builtin = if (is_mod) - try Tag.rem.create(c.arena, operands) + try Tag.signed_remainder.create(c.arena, operands) else try Tag.div_trunc.create(c.arena, operands); diff --git a/src/translate_c/ast.zig b/src/translate_c/ast.zig index 2cab672445..a4e64e1966 100644 --- a/src/translate_c/ast.zig +++ b/src/translate_c/ast.zig @@ -128,8 +128,8 @@ pub const Node = extern union { helpers_promoteIntLiteral, /// @import("std").meta.alignment(value) std_meta_alignment, - /// @rem(lhs, rhs) - rem, + /// @import("std").zig.c_translation.signedRemainder(lhs, rhs) + signed_remainder, /// @divTrunc(lhs, rhs) div_trunc, /// @boolToInt(operand) @@ -310,7 +310,7 @@ pub const Node = extern union { .bit_xor, .bit_xor_assign, .div_trunc, - .rem, + .signed_remainder, .int_cast, .as, .truncate, @@ -1293,9 +1293,10 @@ fn renderNode(c: *Context, node: Node) Allocator.Error!NodeIndex { const payload = node.castTag(.int_cast).?.data; return renderBuiltinCall(c, "@intCast", &.{ payload.lhs, payload.rhs }); }, - .rem => { - const payload = node.castTag(.rem).?.data; - return renderBuiltinCall(c, "@rem", &.{ payload.lhs, payload.rhs }); + .signed_remainder => { + const payload = node.castTag(.signed_remainder).?.data; + const import_node = try renderStdImport(c, &.{ "zig", "c_translation", "signedRemainder" }); + return renderCall(c, import_node, &.{ payload.lhs, payload.rhs }); }, .div_trunc => { const payload = node.castTag(.div_trunc).?.data; @@ -2207,7 +2208,7 @@ fn renderNodeGrouped(c: *Context, node: Node) !NodeIndex { .noreturn_type, .@"anytype", .div_trunc, - .rem, + .signed_remainder, .int_cast, .as, .truncate, diff --git a/test/run_translated_c.zig b/test/run_translated_c.zig index 91e6cc9cfd..147b7544a3 100644 --- a/test/run_translated_c.zig +++ b/test/run_translated_c.zig @@ -1784,4 +1784,16 @@ pub fn addCases(cases: *tests.RunTranslatedCContext) void { \\ return 0; \\} , ""); + + cases.add("Remainder operator with negative integers. Issue #10176", + \\#include + \\int main(void) { + \\ int denominator = -2; + \\ int numerator = 5; + \\ if (numerator % denominator != 1) abort(); + \\ numerator = -5; denominator = 2; + \\ if (numerator % denominator != -1) abort(); + \\ return 0; + \\} + , ""); } diff --git a/test/translate_c.zig b/test/translate_c.zig index b63bbfc57d..445c0a603a 100644 --- a/test/translate_c.zig +++ b/test/translate_c.zig @@ -885,7 +885,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ c = a - b; \\ c = a * b; \\ c = @divTrunc(a, b); - \\ c = @rem(a, b); + \\ c = @import("std").zig.c_translation.signedRemainder(a, b); \\ return 0; \\} \\pub export fn u() c_uint { @@ -2932,9 +2932,9 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ ref.* = @divTrunc(ref.*, @as(c_int, 1)); \\ break :blk ref.*; \\ }); - \\ a = @rem(a, blk: { + \\ a = @import("std").zig.c_translation.signedRemainder(a, blk: { \\ const ref = &a; - \\ ref.* = @rem(ref.*, @as(c_int, 1)); + \\ ref.* = @import("std").zig.c_translation.signedRemainder(ref.*, @as(c_int, 1)); \\ break :blk ref.*; \\ }); \\ b /= blk: {