Commit dce612ac2b

Evan Haas <evan@lagerdata.com>
2021-03-19 07:13:59
translate-c: Ensure assignments are within a block when necessary
Ensures that if an assignment statement is the sole statement within a C if statement, for loop, do loop, or do while loop, then when translated it resides within a block, even though it does not in the original C. Fixes the following invalid translation: `if (1) if (1) 2;` -> `if (true) if (true) _ = @as(c_int, 2);` To this: ```zig if (true) if (true) { _ = @as(c_int, 2); }; ``` Fixes #8159
1 parent 187af14
src/translate_c.zig
@@ -1063,6 +1063,7 @@ fn transStmt(
             const gen_sel = @ptrCast(*const clang.GenericSelectionExpr, stmt);
             return transExpr(c, scope, gen_sel.getResultExpr(), result_used);
         },
+        // When adding new cases here, see comment for maybeBlockify()
         else => {
             return fail(c, error.UnsupportedTranslation, stmt.getBeginLoc(), "TODO implement translation of stmt class {s}", .{@tagName(sc)});
         },
@@ -2242,6 +2243,35 @@ fn transImplicitValueInitExpr(
     return transZeroInitExpr(c, scope, source_loc, ty);
 }
 
+/// If a statement can possibly translate to a Zig assignment (either directly because it's
+/// an assignment in C or indirectly via result assignment to `_`) AND it's the sole statement
+/// in the body of an if statement or loop, then we need to put the statement into its own block.
+/// The `else` case here corresponds to statements that could result in an assignment. If a statement
+/// class never needs a block, add its enum to the top prong.
+fn maybeBlockify(c: *Context, scope: *Scope, stmt: *const clang.Stmt) TransError!Node {
+    switch (stmt.getStmtClass()) {
+        .BreakStmtClass,
+        .CompoundStmtClass,
+        .ContinueStmtClass,
+        .DeclRefExprClass,
+        .DeclStmtClass,
+        .DoStmtClass,
+        .ForStmtClass,
+        .IfStmtClass,
+        .ReturnStmtClass,
+        .NullStmtClass,
+        .WhileStmtClass,
+        => return transStmt(c, scope, stmt, .unused),
+        else => {
+            var block_scope = try Scope.Block.init(c, scope, false);
+            defer block_scope.deinit();
+            const result = try transStmt(c, &block_scope.base, stmt, .unused);
+            try block_scope.statements.append(result);
+            return block_scope.complete(c);
+        },
+    }
+}
+
 fn transIfStmt(
     c: *Context,
     scope: *Scope,
@@ -2259,9 +2289,10 @@ fn transIfStmt(
     const cond_expr = @ptrCast(*const clang.Expr, stmt.getCond());
     const cond = try transBoolExpr(c, &cond_scope.base, cond_expr, .used);
 
-    const then_body = try transStmt(c, scope, stmt.getThen(), .unused);
+    const then_body = try maybeBlockify(c, scope, stmt.getThen());
+
     const else_body = if (stmt.getElse()) |expr|
-        try transStmt(c, scope, expr, .unused)
+        try maybeBlockify(c, scope, expr)
     else
         null;
     return Tag.@"if".create(c.arena, .{ .cond = cond, .then = then_body, .@"else" = else_body });
@@ -2286,7 +2317,7 @@ fn transWhileLoop(
         .parent = scope,
         .id = .loop,
     };
-    const body = try transStmt(c, &loop_scope, stmt.getBody(), .unused);
+    const body = try maybeBlockify(c, &loop_scope, stmt.getBody());
     return Tag.@"while".create(c.arena, .{ .cond = cond, .body = body, .cont_expr = null });
 }
 
@@ -2312,7 +2343,7 @@ fn transDoWhileLoop(
     const if_not_break = switch (cond.tag()) {
         .false_literal => return transStmt(c, scope, stmt.getBody(), .unused),
         .true_literal => {
-            const body_node = try transStmt(c, scope, stmt.getBody(), .unused);
+            const body_node = try maybeBlockify(c, scope, stmt.getBody());
             return Tag.while_true.create(c.arena, body_node);
         },
         else => try Tag.if_not_break.create(c.arena, cond),
@@ -2388,7 +2419,7 @@ fn transForLoop(
     else
         null;
 
-    const body = try transStmt(c, &loop_scope, stmt.getBody(), .unused);
+    const body = try maybeBlockify(c, &loop_scope, stmt.getBody());
     const while_node = try Tag.@"while".create(c.arena, .{ .cond = cond, .body = body, .cont_expr = cont_expr });
     if (block_scope) |*bs| {
         try bs.statements.append(while_node);
test/run_translated_c.zig
@@ -1244,4 +1244,18 @@ pub fn addCases(cases: *tests.RunTranslatedCContext) void {
         \\    return 0;
         \\}
     , "");
+
+    cases.add("convert single-statement bodies into blocks for if/else/for/while. issue #8159",
+        \\#include <stdlib.h>
+        \\int foo() { return 1; }
+        \\int main(void) {
+        \\    int i = 0;
+        \\    if (i == 0) if (i == 0) if (i != 0) i = 1;
+        \\    if (i != 0) i = 1; else if (i == 0) if (i == 0) i += 1;
+        \\    for (; i < 10;) for (; i < 10;) i++;
+        \\    while (i == 100) while (i == 100) foo();
+        \\    if (0) do do "string"; while(1); while(1);
+        \\    return 0;
+        \\}
+    , "");
 }
test/translate_c.zig
@@ -1934,7 +1934,9 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
     , &[_][]const u8{
         \\pub export fn foo() c_int {
         \\    var a: c_int = 5;
-        \\    while (true) a = 2;
+        \\    while (true) {
+        \\        a = 2;
+        \\    }
         \\    while (true) {
         \\        var a_1: c_int = 4;
         \\        a_1 = 9;
@@ -1947,7 +1949,9 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
         \\        var a_1: c_int = 2;
         \\        a_1 = 12;
         \\    }
-        \\    while (true) a = 7;
+        \\    while (true) {
+        \\        a = 7;
+        \\    }
         \\    return 0;
         \\}
     });
@@ -2008,7 +2012,9 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
         \\}
     , &[_][]const u8{
         \\pub export fn bar() c_int {
-        \\    if ((if (true) @as(c_int, 5) else if (true) @as(c_int, 4) else @as(c_int, 6)) != 0) _ = @as(c_int, 2);
+        \\    if ((if (true) @as(c_int, 5) else if (true) @as(c_int, 4) else @as(c_int, 6)) != 0) {
+        \\        _ = @as(c_int, 2);
+        \\    }
         \\    return if (true) @as(c_int, 5) else if (true) @as(c_int, 4) else @as(c_int, 6);
         \\}
     });
@@ -2389,7 +2395,9 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
         \\pub const yes = [*c]u8;
         \\pub export fn foo() void {
         \\    var a: yes = undefined;
-        \\    if (a != null) _ = @as(c_int, 2);
+        \\    if (a != null) {
+        \\        _ = @as(c_int, 2);
+        \\    }
         \\}
     });