Commit ea4a25287e

Evan Haas <evan@lagerdata.com>
2021-06-09 02:09:42
translate-c: better support for static local variables
Don't move static local variables into the top-level scope since this can cause name clashes if subsequently-defined variables or parameters in different scopes share the name. Instead, use a variable within a struct so that the variable's lexical scope does not change. This solution was suggested by @LemonBoy Note that a similar name-shadowing problem exists with `extern` variables declared within block scope, but a different solution will be needed since they do need to be moved to the top-level scope and we can't rename them.
1 parent 44cdafd
src/translate_c/ast.zig
@@ -60,6 +60,8 @@ pub const Node = extern union {
         array_access,
         call,
         var_decl,
+        /// const name = struct { init }
+        static_local_var,
         func,
         warning,
         /// All enums are non-exhaustive
@@ -366,7 +368,7 @@ pub const Node = extern union {
                 .array_type, .null_sentinel_array_type => Payload.Array,
                 .arg_redecl, .alias, .fail_decl => Payload.ArgRedecl,
                 .log2_int_type => Payload.Log2IntType,
-                .var_simple, .pub_var_simple => Payload.SimpleVarDecl,
+                .var_simple, .pub_var_simple, .static_local_var => Payload.SimpleVarDecl,
                 .pub_enum_redecl, .enum_redecl => Payload.EnumRedecl,
                 .array_filler => Payload.ArrayFiller,
                 .pub_inline_fn => Payload.PubInlineFn,
@@ -1209,6 +1211,35 @@ fn renderNode(c: *Context, node: Node) Allocator.Error!NodeIndex {
                 },
             });
         },
+        .static_local_var => {
+            const payload = node.castTag(.static_local_var).?.data;
+
+            const const_tok = try c.addToken(.keyword_const, "const");
+            _ = try c.addIdentifier(payload.name);
+            _ = try c.addToken(.equal, "=");
+
+            const kind_tok = try c.addToken(.keyword_struct, "struct");
+            _ = try c.addToken(.l_brace, "{");
+
+            const container_def = try c.addNode(.{
+                .tag = .container_decl_two_trailing,
+                .main_token = kind_tok,
+                .data = .{
+                    .lhs = try renderNode(c, payload.init),
+                    .rhs = 0,
+                },
+            });
+            _ = try c.addToken(.r_brace, "}");
+
+            return c.addNode(.{
+                .tag = .simple_var_decl,
+                .main_token = const_tok,
+                .data = .{
+                    .lhs = 0,
+                    .rhs = container_def,
+                },
+            });
+        },
         .var_decl => return renderVar(c, node),
         .arg_redecl, .alias => {
             const payload = @fieldParentPtr(Payload.ArgRedecl, "base", node.ptr_otherwise).data;
@@ -2276,6 +2307,7 @@ fn renderNodeGrouped(c: *Context, node: Node) !NodeIndex {
         .div_exact,
         .offset_of,
         .shuffle,
+        .static_local_var,
         => {
             // no grouping needed
             return renderNode(c, node);
src/clang.zig
@@ -1002,6 +1002,9 @@ pub const VarDecl = opaque {
 
     pub const getTypeSourceInfo_getType = ZigClangVarDecl_getTypeSourceInfo_getType;
     extern fn ZigClangVarDecl_getTypeSourceInfo_getType(*const VarDecl) QualType;
+
+    pub const isStaticLocal = ZigClangVarDecl_isStaticLocal;
+    extern fn ZigClangVarDecl_isStaticLocal(*const VarDecl) bool;
 };
 
 pub const VectorType = opaque {
src/translate_c.zig
@@ -72,6 +72,11 @@ const Scope = struct {
         /// so that the return expression can be cast, if necessary
         return_type: ?clang.QualType = null,
 
+        /// C static local variables are wrapped in a block-local struct. The struct
+        /// is named after the (mangled) variable name, the Zig variable within the
+        /// struct itself is given this name.
+        const StaticInnerName = "static";
+
         fn init(c: *Context, parent: *Scope, labeled: bool) !Block {
             var blk = Block{
                 .base = .{
@@ -1738,15 +1743,13 @@ fn transDeclStmtOne(
             const name = try c.str(@ptrCast(*const clang.NamedDecl, var_decl).getName_bytes_begin());
             const mangled_name = try block_scope.makeMangledName(c, name);
 
-            switch (var_decl.getStorageClass()) {
-                .Extern, .Static => {
-                    // This is actually a global variable, put it in the global scope and reference it.
-                    // `_ = mangled_name;`
-                    return visitVarDecl(c, var_decl, mangled_name);
-                },
-                else => {},
+            if (var_decl.getStorageClass() == .Extern) {
+                // This is actually a global variable, put it in the global scope and reference it.
+                // `_ = mangled_name;`
+                return visitVarDecl(c, var_decl, mangled_name);
             }
 
+            const is_static_local = var_decl.isStaticLocal();
             const is_const = qual_type.isConstQualified();
 
             const loc = decl.getLocation();
@@ -1757,24 +1760,30 @@ fn transDeclStmtOne(
                     try transStringLiteralInitializer(c, scope, @ptrCast(*const clang.StringLiteral, expr), type_node)
                 else
                     try transExprCoercing(c, scope, expr, .used)
+            else if (is_static_local)
+                try Tag.std_mem_zeroes.create(c.arena, type_node)
             else
                 Tag.undefined_literal.init();
             if (!qualTypeIsBoolean(qual_type) and isBoolRes(init_node)) {
                 init_node = try Tag.bool_to_int.create(c.arena, init_node);
             }
 
-            const node = try Tag.var_decl.create(c.arena, .{
+            const var_name: []const u8 = if (is_static_local) Scope.Block.StaticInnerName else mangled_name;
+            var node = try Tag.var_decl.create(c.arena, .{
                 .is_pub = false,
                 .is_const = is_const,
                 .is_extern = false,
                 .is_export = false,
-                .is_threadlocal = false,
+                .is_threadlocal = var_decl.getTLSKind() != .None,
                 .linksection_string = null,
                 .alignment = zigAlignment(var_decl.getAlignedAttribute(c.clang_context)),
-                .name = mangled_name,
+                .name = var_name,
                 .type = type_node,
                 .init = init_node,
             });
+            if (is_static_local) {
+                node = try Tag.static_local_var.create(c.arena, .{ .name = mangled_name, .init = node });
+            }
             try block_scope.statements.append(node);
 
             const cleanup_attr = var_decl.getCleanupAttribute();
@@ -1834,7 +1843,18 @@ fn transDeclRefExpr(
     const value_decl = expr.getDecl();
     const name = try c.str(@ptrCast(*const clang.NamedDecl, value_decl).getName_bytes_begin());
     const mangled_name = scope.getAlias(name);
-    return Tag.identifier.create(c.arena, mangled_name);
+    var ref_expr = try Tag.identifier.create(c.arena, mangled_name);
+
+    if (@ptrCast(*const clang.Decl, value_decl).getKind() == .Var) {
+        const var_decl = @ptrCast(*const clang.VarDecl, value_decl);
+        if (var_decl.isStaticLocal()) {
+            ref_expr = try Tag.field_access.create(c.arena, .{
+                .lhs = ref_expr,
+                .field_name = Scope.Block.StaticInnerName,
+            });
+        }
+    }
+    return ref_expr;
 }
 
 fn transImplicitCastExpr(
src/zig_clang.cpp
@@ -2458,6 +2458,11 @@ enum ZigClangStorageClass ZigClangVarDecl_getStorageClass(const struct ZigClangV
     return (ZigClangStorageClass)casted->getStorageClass();
 }
 
+bool ZigClangVarDecl_isStaticLocal(const struct ZigClangVarDecl *self) {
+    auto casted = reinterpret_cast<const clang::VarDecl *>(self);
+    return casted->isStaticLocal();
+}
+
 enum ZigClangBuiltinTypeKind ZigClangBuiltinType_getKind(const struct ZigClangBuiltinType *self) {
     auto casted = reinterpret_cast<const clang::BuiltinType *>(self);
     return (ZigClangBuiltinTypeKind)casted->getKind();
src/zig_clang.h
@@ -1072,6 +1072,7 @@ ZIG_EXTERN_C bool ZigClangVarDecl_hasInit(const struct ZigClangVarDecl *);
 ZIG_EXTERN_C const struct ZigClangAPValue *ZigClangVarDecl_evaluateValue(const struct ZigClangVarDecl *);
 ZIG_EXTERN_C struct ZigClangQualType ZigClangVarDecl_getTypeSourceInfo_getType(const struct ZigClangVarDecl *);
 ZIG_EXTERN_C enum ZigClangStorageClass ZigClangVarDecl_getStorageClass(const struct ZigClangVarDecl *self);
+ZIG_EXTERN_C bool ZigClangVarDecl_isStaticLocal(const struct ZigClangVarDecl *self);
 
 ZIG_EXTERN_C bool ZigClangSourceLocation_eq(struct ZigClangSourceLocation a, struct ZigClangSourceLocation b);
 
test/run_translated_c.zig
@@ -1550,4 +1550,42 @@ pub fn addCases(cases: *tests.RunTranslatedCContext) void {
         \\    if(FORCE_UINT != 0xffffffff) abort();
         \\}
     , "");
+
+    cases.add("block-scope static variable shadows function parameter. Issue #8208",
+        \\#include <stdlib.h>
+        \\int func1(int foo) { return foo + 1; }
+        \\int func2(void) {
+        \\    static int foo = 5;
+        \\    return foo++;
+        \\}
+        \\int main(void) {
+        \\    if (func1(42) != 43) abort();
+        \\    if (func2() != 5) abort();
+        \\    if (func2() != 6) abort();
+        \\    return 0;
+        \\}
+    , "");
+
+    cases.add("nested same-name static locals",
+        \\#include <stdlib.h>
+        \\int func(int val) {
+        \\    static int foo;
+        \\    if (foo != val) abort();
+        \\    {
+        \\        foo += 1;
+        \\        static int foo = 2;
+        \\        if (foo != val + 2) abort();
+        \\        foo += 1;
+        \\    }
+        \\    return foo;
+        \\}
+        \\int main(void) {
+        \\    int foo = 1;
+        \\    if (func(0) != 1) abort();
+        \\    if (func(1) != 2) abort();
+        \\    if (func(2) != 3) abort();
+        \\    if (foo != 1) abort();
+        \\    return 0;
+        \\}
+    , "");
 }
test/translate_c.zig
@@ -286,15 +286,17 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
         \\}
     });
 
-    cases.add("extern variable in block scope",
+    cases.add("static variable in block scope",
         \\float bar;
         \\int foo() {
         \\    _Thread_local static int bar = 2;
         \\}
     , &[_][]const u8{
         \\pub export var bar: f32 = @import("std").mem.zeroes(f32);
-        \\threadlocal var bar_1: c_int = 2;
         \\pub export fn foo() c_int {
+        \\    const bar_1 = struct {
+        \\        threadlocal var static: c_int = 2;
+        \\    };
         \\    return 0;
         \\}
     });
@@ -816,8 +818,11 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
         \\    static const char v2[] = "2.2.2";
         \\}
     , &[_][]const u8{
-        \\const v2: [5:0]u8 = "2.2.2".*;
-        \\pub export fn foo() void {}
+        \\pub export fn foo() void {
+        \\    const v2 = struct {
+        \\        const static: [5:0]u8 = "2.2.2".*;
+        \\    };
+        \\}
     });
 
     cases.add("simple function definition",
@@ -3595,4 +3600,23 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
         \\    return bar(@as(c_int, 1), @as(c_int, 2));
         \\}
     });
+
+    cases.add("static local variable zero-initialized if no initializer",
+        \\struct FOO {int x; int y;};
+        \\int bar(void) {
+        \\    static struct FOO foo;
+        \\    return foo.x;
+        \\}
+    , &[_][]const u8{
+        \\pub const struct_FOO = extern struct {
+        \\    x: c_int,
+        \\    y: c_int,
+        \\};
+        \\pub export fn bar() c_int {
+        \\    const foo = struct {
+        \\        var static: struct_FOO = @import("std").mem.zeroes(struct_FOO);
+        \\    };
+        \\    return foo.static.x;
+        \\}
+    });
 }