From d98f09e4f67fb2848be6052466db035450326605 Mon Sep 17 00:00:00 2001
From: Evan Haas <evan@lagerdata.com>
Date: Thu, 11 Feb 2021 09:43:30 -0800
Subject: [PATCH] translate-c: comma operator should introduce a new scope

This prevents inadvertent side-effects when an expression is not evaluated
due to boolean short-circuiting

Fixes #7989
---
 src/translate_c.zig       | 33 ++++++++++++++-------------------
 test/run_translated_c.zig | 13 +++++++++++++
 test/translate_c.zig      | 32 ++++++++++++++++++++++----------
 3 files changed, 49 insertions(+), 29 deletions(-)

diff --git a/src/translate_c.zig b/src/translate_c.zig
index 82a7695c13..4dc9453c85 100644
--- a/src/translate_c.zig
+++ b/src/translate_c.zig
@@ -1438,30 +1438,25 @@ fn transBinaryOperator(
     switch (op) {
         .Assign => return try transCreateNodeAssign(rp, scope, result_used, stmt.getLHS(), stmt.getRHS()),
         .Comma => {
-            const block_scope = try scope.findBlockScope(rp.c);
-            const expr = block_scope.base.parent == scope;
-            const lparen = if (expr) try appendToken(rp.c, .LParen, "(") else undefined;
+            var block_scope = try Scope.Block.init(rp.c, scope, true);
+            const lparen = try appendToken(rp.c, .LParen, "(");
 
             const lhs = try transExpr(rp, &block_scope.base, stmt.getLHS(), .unused, .r_value);
             try block_scope.statements.append(lhs);
 
             const rhs = try transExpr(rp, &block_scope.base, stmt.getRHS(), .used, .r_value);
-            if (expr) {
-                _ = try appendToken(rp.c, .Semicolon, ";");
-                const break_node = try transCreateNodeBreak(rp.c, block_scope.label, rhs);
-                try block_scope.statements.append(&break_node.base);
-                const block_node = try block_scope.complete(rp.c);
-                const rparen = try appendToken(rp.c, .RParen, ")");
-                const grouped_expr = try rp.c.arena.create(ast.Node.GroupedExpression);
-                grouped_expr.* = .{
-                    .lparen = lparen,
-                    .expr = block_node,
-                    .rparen = rparen,
-                };
-                return maybeSuppressResult(rp, scope, result_used, &grouped_expr.base);
-            } else {
-                return maybeSuppressResult(rp, scope, result_used, rhs);
-            }
+            _ = try appendToken(rp.c, .Semicolon, ";");
+            const break_node = try transCreateNodeBreak(rp.c, block_scope.label, rhs);
+            try block_scope.statements.append(&break_node.base);
+            const block_node = try block_scope.complete(rp.c);
+            const rparen = try appendToken(rp.c, .RParen, ")");
+            const grouped_expr = try rp.c.arena.create(ast.Node.GroupedExpression);
+            grouped_expr.* = .{
+                .lparen = lparen,
+                .expr = block_node,
+                .rparen = rparen,
+            };
+            return maybeSuppressResult(rp, scope, result_used, &grouped_expr.base);
         },
         .Div => {
             if (cIsSignedInteger(qt)) {
diff --git a/test/run_translated_c.zig b/test/run_translated_c.zig
index e28bdc96f0..b8af201e36 100644
--- a/test/run_translated_c.zig
+++ b/test/run_translated_c.zig
@@ -909,4 +909,17 @@ pub fn addCases(cases: *tests.RunTranslatedCContext) void {
         \\    return 1 != 1;
         \\}
     , "");
+
+    cases.add("Comma operator should create new scope; issue #7989",
+        \\#include <stdlib.h>
+        \\#include <stdio.h>
+        \\int main(void) {
+        \\    if (1 || (abort(), 1)) {}
+        \\    if (0 && (1, printf("do not print\n"))) {}
+        \\    int x = 0;
+        \\    x = (x = 3, 4, x + 1);
+        \\    if (x != 4) abort();
+        \\    return 0;
+        \\}
+    , "");
 }
diff --git a/test/translate_c.zig b/test/translate_c.zig
index 03ca87d5f6..2097e17323 100644
--- a/test/translate_c.zig
+++ b/test/translate_c.zig
@@ -1723,11 +1723,17 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
         \\}
     , &[_][]const u8{
         \\pub export fn foo() c_int {
-        \\    _ = @as(c_int, 2);
-        \\    _ = @as(c_int, 4);
-        \\    _ = @as(c_int, 2);
-        \\    _ = @as(c_int, 4);
-        \\    return @as(c_int, 6);
+        \\    _ = (blk: {
+        \\        _ = @as(c_int, 2);
+        \\        break :blk @as(c_int, 4);
+        \\    });
+        \\    return (blk: {
+        \\        _ = (blk_1: {
+        \\            _ = @as(c_int, 2);
+        \\            break :blk_1 @as(c_int, 4);
+        \\        });
+        \\        break :blk @as(c_int, 6);
+        \\    });
         \\}
     });
 
@@ -1774,8 +1780,10 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
         \\    while (true) {
         \\        var a_1: c_int = 4;
         \\        a_1 = 9;
-        \\        _ = @as(c_int, 6);
-        \\        return a_1;
+        \\        return (blk: {
+        \\            _ = @as(c_int, 6);
+        \\            break :blk a_1;
+        \\        });
         \\    }
         \\    while (true) {
         \\        var a_1: c_int = 2;
@@ -1805,9 +1813,13 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
         \\        var b: c_int = 4;
         \\        while ((i + @as(c_int, 2)) != 0) : (i = 2) {
         \\            var a: c_int = 2;
-        \\            a = 6;
-        \\            _ = @as(c_int, 5);
-        \\            _ = @as(c_int, 7);
+        \\            _ = (blk: {
+        \\                _ = (blk_1: {
+        \\                    a = 6;
+        \\                    break :blk_1 @as(c_int, 5);
+        \\                });
+        \\                break :blk @as(c_int, 7);
+        \\            });
         \\        }
         \\    }
         \\    var i: u8 = @bitCast(u8, @truncate(i8, @as(c_int, 2)));