Commit bcf15e39e2d4e2243f475852aca7749e40a70fbd | zig

author	Andrew Kelley <andrew@ziglang.org> 2021-05-11 14:17:52-07:00
committer	Andrew Kelley <andrew@ziglang.org> 2021-05-11 14:17:52-07:00
log	bcf15e39e2d4e2243f475852aca7749e40a70fbd
tree	367c697ea25a0649a263d13131ffaf0ec3f82ad4
parent	dae22a0a1f13cc963e96cd704941eed29b8dde27

stage2: add `owns_tv` flag to `Module.Decl`

Decl objects need to know whether they are the owner of the Type/Value
associated with them, in order to decide whether to destroy the
associated Namespace, Fn, or Var when cleaning up.

6 files changed, 128 insertions(+), 59 deletions(-)

BRANCH_TODO
src
- Module.zig
- Sema.zig
- codegen
  - c.zig
- link
  - C
    zig.h
test
- stage2
  - cbe.zig

BRANCH_TODO-10

...	@@ -1,13 +1,3 @@	...	@@ -1,13 +1,3 @@
1	* The next problem is that when trying to deinitialize everything, when we
2	deinit a Decl that is the owner of a Namespace, there may still be other Decl
3	objects that reference that Namespace. They want to check if they are the owner
4	in order to find out if they should destroy it. But they can't check if they are
5	the owner because the owner_decl field is destroyed.
6	So there's a memory management problem to solve. We could easily solve this with
7	ref counting or whatever but the goal is to not introduce extra overhead / unnecessary
8	fields just to help figure out how to free stuff. So come up with some way to make
9	this sound, and easily debuggable when something goes wrong.
10
11	* get stage2 tests passing	1	* get stage2 tests passing
12	* modify stage2 tests so that only 1 uses _start and the rest use	2	* modify stage2 tests so that only 1 uses _start and the rest use
13	pub fn main	3	pub fn main

src/Module.zig+57-29

 @@ -227,6 +227,10 @@ pub const Decl = struct {
     },
     /// Whether `typed_value`, `align_val`, and `linksection_val` are populated.
     has_tv: bool,
+    /// If `true` it means the `Decl` is the resource owner of the type/value associated
+    /// with it. That means when `Decl` is destroyed, the cleanup code should additionally
+    /// check if the value owns a `Namespace`, and destroy that too.
+    owns_tv: bool,
     /// This flag is set when this Decl is added to `Module.deletion_set`, and cleared
     /// when removed.
     deletion_flag: bool,
 @@ -278,9 +282,7 @@ pub const Decl = struct {
         decl.clearName(gpa);
         if (decl.has_tv) {
             if (decl.getInnerNamespace()) |namespace| {
-                if (namespace.getDecl() == decl) {
+                namespace.clearDecls(module);
-                    namespace.clearDecls(module);
-                }
             }
             decl.clearValues(gpa);
         }
 @@ -307,6 +309,7 @@ pub const Decl = struct {
             arena_state.promote(gpa).deinit();
             decl.value_arena = null;
             decl.has_tv = false;
+            decl.owns_tv = false;
         }
     }
 @@ -441,36 +444,36 @@ pub const Decl = struct {
     /// If the Decl has a value and it is a struct, return it,
     /// otherwise null.
     pub fn getStruct(decl: *Decl) ?*Struct {
-        if (!decl.has_tv) return null;
+        if (!decl.owns_tv) return null;
         const ty = (decl.val.castTag(.ty) orelse return null).data;
         const struct_obj = (ty.castTag(.@"struct") orelse return null).data;
-        if (struct_obj.owner_decl != decl) return null;
+        assert(struct_obj.owner_decl == decl);
         return struct_obj;
     }
     /// If the Decl has a value and it is a union, return it,
     /// otherwise null.
     pub fn getUnion(decl: *Decl) ?*Union {
-        if (!decl.has_tv) return null;
+        if (!decl.owns_tv) return null;
         const ty = (decl.val.castTag(.ty) orelse return null).data;
         const union_obj = (ty.cast(Type.Payload.Union) orelse return null).data;
-        if (union_obj.owner_decl != decl) return null;
+        assert(union_obj.owner_decl == decl);
         return union_obj;
     }
     /// If the Decl has a value and it is a function, return it,
     /// otherwise null.
     pub fn getFunction(decl: *Decl) ?*Fn {
-        if (!decl.has_tv) return null;
+        if (!decl.owns_tv) return null;
         const func = (decl.val.castTag(.function) orelse return null).data;
-        if (func.owner_decl != decl) return null;
+        assert(func.owner_decl == decl);
         return func;
     }
     pub fn getVariable(decl: *Decl) ?*Var {
-        if (!decl.has_tv) return null;
+        if (!decl.owns_tv) return null;
         const variable = (decl.val.castTag(.variable) orelse return null).data;
-        if (variable.owner_decl != decl) return null;
+        assert(variable.owner_decl == decl);
         return variable;
     }
 @@ -478,29 +481,28 @@ pub const Decl = struct {
     /// enum, or opaque.
     /// Only returns it if the Decl is the owner.
     pub fn getInnerNamespace(decl: *Decl) ?*Scope.Namespace {
-        if (!decl.has_tv) return null;
+        if (!decl.owns_tv) return null;
         const ty = (decl.val.castTag(.ty) orelse return null).data;
         switch (ty.tag()) {
             .@"struct" => {
                 const struct_obj = ty.castTag(.@"struct").?.data;
-                if (struct_obj.owner_decl != decl) return null;
+                assert(struct_obj.owner_decl == decl);
                 return &struct_obj.namespace;
             },
             .enum_full => {
                 const enum_obj = ty.castTag(.enum_full).?.data;
-                if (enum_obj.owner_decl != decl) return null;
+                assert(enum_obj.owner_decl == decl);
                 return &enum_obj.namespace;
             },
             .empty_struct => {
-                // design flaw, can't verify the owner is this decl
+                return ty.castTag(.empty_struct).?.data;
-                @panic("TODO can't implement getInnerNamespace for this type");
             },
             .@"opaque" => {
                 @panic("TODO opaque types");
             },
             .@"union", .union_tagged => {
                 const union_obj = ty.cast(Type.Payload.Union).?.data;
-                if (union_obj.owner_decl != decl) return null;
+                assert(union_obj.owner_decl == decl);
                 return &union_obj.namespace;
             },
 @@ -554,7 +556,11 @@ pub const Decl = struct {
             .complete,
             .outdated,
-            => true,
+            => {
+                if (!decl.owns_tv)
+                    return false;
+                return decl.ty.hasCodeGenBits();
+            },
         };
     }
 };
 @@ -2838,6 +2844,7 @@ pub fn semaFile(mod: *Module, file: *Scope.File) InnerError!void {
     new_decl.ty = struct_ty;
     new_decl.val = struct_val;
     new_decl.has_tv = true;
+    new_decl.owns_tv = true;
     new_decl.analysis = .complete;
     new_decl.generation = mod.generation;
 @@ -2948,7 +2955,7 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool {
     errdefer decl_arena.deinit();
     const decl_arena_state = try decl_arena.allocator.create(std.heap.ArenaAllocator.State);
-    if (decl_tv.val.tag() == .function) {
+    if (decl_tv.val.castTag(.function)) |fn_payload| {
         var prev_type_has_bits = false;
         var prev_is_inline = false;
         var type_changed = true;
 @@ -2956,10 +2963,8 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool {
         if (decl.has_tv) {
             prev_type_has_bits = decl.ty.hasCodeGenBits();
             type_changed = !decl.ty.eql(decl_tv.ty);
-            if (decl.val.castTag(.function)) |payload| {
+            if (decl.getFunction()) |prev_func| {
-                const prev_func = payload.data;
                 prev_is_inline = prev_func.state == .inline_only;
-                prev_func.deinit(gpa);
             }
             decl.clearValues(gpa);
         }
 @@ -2969,6 +2974,7 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool {
         decl.align_val = try align_val.copy(&decl_arena.allocator);
         decl.linksection_val = try linksection_val.copy(&decl_arena.allocator);
         decl.has_tv = true;
+        decl.owns_tv = fn_payload.data.owner_decl == decl;
         decl_arena_state.* = decl_arena.state;
         decl.value_arena = decl_arena_state;
         decl.analysis = .complete;
 @@ -3004,6 +3010,25 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool {
             decl.clearValues(gpa);
         }
+        decl.owns_tv = false;
+        var queue_linker_work = false;
+        if (decl_tv.val.castTag(.variable)) |payload| {
+            const variable = payload.data;
+            if (variable.owner_decl == decl) {
+                decl.owns_tv = true;
+                queue_linker_work = true;
+                const copied_init = try variable.init.copy(&decl_arena.allocator);
+                variable.init = copied_init;
+            }
+        } else if (decl_tv.val.castTag(.extern_fn)) |payload| {
+            const owner_decl = payload.data;
+            if (decl == owner_decl) {
+                decl.owns_tv = true;
+                queue_linker_work = true;
+            }
+        }
         decl.ty = try decl_tv.ty.copy(&decl_arena.allocator);
         decl.val = try decl_tv.val.copy(&decl_arena.allocator);
         decl.align_val = try align_val.copy(&decl_arena.allocator);
 @@ -3014,13 +3039,7 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool {
         decl.analysis = .complete;
         decl.generation = mod.generation;
-        if (decl.is_exported) {
+        if (queue_linker_work and decl.ty.hasCodeGenBits()) {
-            const export_src = src; // TODO point to the export token
-            // The scope needs to have the decl in it.
-            try mod.analyzeExport(&block_scope.base, export_src, mem.spanZ(decl.name), decl);
-        }
-        if (decl.val.tag() == .extern_fn) {
             try mod.comp.bin_file.allocateDeclIndexes(decl);
             try mod.comp.work_queue.writeItem(.{ .codegen_decl = decl });
 @@ -3029,6 +3048,13 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool {
             }
         }
+        if (decl.is_exported) {
+            const export_src = src; // TODO point to the export token
+            // The scope needs to have the decl in it.
+            try mod.analyzeExport(&block_scope.base, export_src, mem.spanZ(decl.name), decl);
+        }
         return type_changed;
     }
 }
 @@ -3531,6 +3557,7 @@ fn allocateNewDecl(mod: *Module, namespace: *Scope.Namespace, src_node: ast.Node
         .src_node = src_node,
         .src_line = undefined,
         .has_tv = false,
+        .owns_tv = false,
         .ty = undefined,
         .val = undefined,
         .align_val = undefined,
 @@ -3779,6 +3806,7 @@ pub fn createAnonymousDeclNamed(
     new_decl.ty = typed_value.ty;
     new_decl.val = typed_value.val;
     new_decl.has_tv = true;
+    new_decl.owns_tv = true;
     new_decl.analysis = .complete;
     new_decl.generation = mod.generation;

src/Sema.zig+1-1

...	@@ -5867,7 +5867,7 @@ fn zirVarExtended(	...	@@ -5867,7 +5867,7 @@ fn zirVarExtended(
5867	extra_index += 1;	5867	extra_index += 1;
5868	const init_tv = try sema.resolveInstConst(block, init_src, init_ref);	5868	const init_tv = try sema.resolveInstConst(block, init_src, init_ref);
5869	break :blk init_tv.val;	5869	break :blk init_tv.val;
5870	} else Value.initTag(.null_value);	5870	} else Value.initTag(.unreachable_value);
5871		5871
5872	if (!var_ty.isValidVarType(small.is_extern)) {	5872	if (!var_ty.isValidVarType(small.is_extern)) {
5873	return sema.mod.fail(&block.base, mut_src, "variable of type '{}' must be const", .{	5873	return sema.mod.fail(&block.base, mut_src, "variable of type '{}' must be const", .{

src/codegen/c.zig+51-4

 @@ -30,6 +30,7 @@ pub const CValue = union(enum) {
     arg: usize,
     /// By-value
     decl: *Decl,
+    decl_ref: *Decl,
 };
 pub const CValueMap = std.AutoHashMap(*Inst, CValue);
 @@ -117,6 +118,7 @@ pub const Object = struct {
             .constant => |inst| return o.dg.renderValue(w, inst.ty, inst.value().?),
             .arg => |i| return w.print("a{d}", .{i}),
             .decl => |decl| return w.writeAll(mem.span(decl.name)),
+            .decl_ref => |decl| return w.print("&{s}", .{decl.name}),
         }
     }
 @@ -528,13 +530,17 @@ pub const DeclGen = struct {
         }
     }
-    fn functionIsGlobal(dg: *DeclGen, tv: TypedValue) bool {
+    fn declIsGlobal(dg: *DeclGen, tv: TypedValue) bool {
         switch (tv.val.tag()) {
             .extern_fn => return true,
             .function => {
                 const func = tv.val.castTag(.function).?.data;
                 return dg.module.decl_exports.contains(func.owner_decl);
             },
+            .variable => {
+                const variable = tv.val.castTag(.variable).?.data;
+                return dg.module.decl_exports.contains(variable.owner_decl);
+            },
             else => unreachable,
         }
     }
 @@ -549,7 +555,7 @@ pub fn genDecl(o: *Object) !void {
         .val = o.dg.decl.val,
     };
     if (tv.val.castTag(.function)) |func_payload| {
-        const is_global = o.dg.functionIsGlobal(tv);
+        const is_global = o.dg.declIsGlobal(tv);
         const fwd_decl_writer = o.dg.fwd_decl.writer();
         if (is_global) {
             try fwd_decl_writer.writeAll("ZIG_EXTERN_C ");
 @@ -570,6 +576,30 @@ pub fn genDecl(o: *Object) !void {
         try writer.writeAll("ZIG_EXTERN_C ");
         try o.dg.renderFunctionSignature(writer, true);
         try writer.writeAll(";\n");
+    } else if (tv.val.castTag(.variable)) |var_payload| {
+        const variable: *Module.Var = var_payload.data;
+        const is_global = o.dg.declIsGlobal(tv);
+        const fwd_decl_writer = o.dg.fwd_decl.writer();
+        if (is_global or variable.is_extern) {
+            try fwd_decl_writer.writeAll("ZIG_EXTERN_C ");
+        }
+        if (variable.is_threadlocal) {
+            try fwd_decl_writer.writeAll("zig_threadlocal ");
+        }
+        try o.dg.renderType(fwd_decl_writer, o.dg.decl.ty);
+        const decl_name = mem.span(o.dg.decl.name);
+        try fwd_decl_writer.print(" {s};\n", .{decl_name});
+        try o.indent_writer.insertNewline();
+        const w = o.writer();
+        try o.dg.renderType(w, o.dg.decl.ty);
+        try w.print(" {s} = ", .{decl_name});
+        if (variable.init.tag() != .unreachable_value) {
+            try o.dg.renderValue(w, tv.ty, variable.init);
+        }
+        try w.writeAll(";");
+        try o.indent_writer.insertNewline();
     } else {
         const writer = o.writer();
         try writer.writeAll("static ");
 @@ -598,7 +628,7 @@ pub fn genHeader(dg: *DeclGen) error{ AnalysisFail, OutOfMemory }!void {
     switch (tv.ty.zigTypeTag()) {
         .Fn => {
-            const is_global = dg.functionIsGlobal(tv);
+            const is_global = dg.declIsGlobal(tv);
             if (is_global) {
                 try writer.writeAll("ZIG_EXTERN_C ");
             }
 @@ -696,7 +726,7 @@ pub fn genBody(o: *Object, body: ir.Body) error{ AnalysisFail, OutOfMemory }!voi
             .wrap_errunion_err => try genWrapErrUnionErr(o, inst.castTag(.wrap_errunion_err).?),
             .br_block_flat => return o.dg.fail(.{ .node_offset = 0 }, "TODO: C backend: implement codegen for br_block_flat", .{}),
             .ptrtoint => return o.dg.fail(.{ .node_offset = 0 }, "TODO: C backend: implement codegen for ptrtoint", .{}),
-            .varptr => return o.dg.fail(.{ .node_offset = 0 }, "TODO: C backend: implement codegen for varptr", .{}),
+            .varptr => try genVarPtr(o, inst.castTag(.varptr).?),
             .floatcast => return o.dg.fail(.{ .node_offset = 0 }, "TODO: C backend: implement codegen for floatcast", .{}),
         };
         switch (result_value) {
 @@ -709,6 +739,10 @@ pub fn genBody(o: *Object, body: ir.Body) error{ AnalysisFail, OutOfMemory }!voi
     try writer.writeAll("}");
 }
+fn genVarPtr(o: *Object, inst: *Inst.VarPtr) !CValue {
+    return CValue{ .decl_ref = inst.variable.owner_decl };
+}
 fn genAlloc(o: *Object, alloc: *Inst.NoOp) !CValue {
     const writer = o.writer();
 @@ -743,6 +777,12 @@ fn genLoad(o: *Object, inst: *Inst.UnOp) !CValue {
             try o.writeCValue(writer, wrapped);
             try writer.writeAll(";\n");
         },
+        .decl_ref => |decl| {
+            const wrapped: CValue = .{ .decl = decl };
+            try writer.writeAll(" = ");
+            try o.writeCValue(writer, wrapped);
+            try writer.writeAll(";\n");
+        },
         else => {
             try writer.writeAll(" = *");
             try o.writeCValue(writer, operand);
 @@ -791,6 +831,13 @@ fn genStore(o: *Object, inst: *Inst.BinOp) !CValue {
             try o.writeCValue(writer, src_val);
             try writer.writeAll(";\n");
         },
+        .decl_ref => |decl| {
+            const dest: CValue = .{ .decl = decl };
+            try o.writeCValue(writer, dest);
+            try writer.writeAll(" = ");
+            try o.writeCValue(writer, src_val);
+            try writer.writeAll(";\n");
+        },
         else => {
             try writer.writeAll("*");
             try o.writeCValue(writer, dest_ptr);

src/link/C/zig.h+18-14

 @@ -1,25 +1,15 @@
-#if __STDC_VERSION__ >= 199901L
-#include <stdbool.h>
-#else
-#define bool unsigned char
-#define true 1
-#define false 0
-#endif
 #if __STDC_VERSION__ >= 201112L
 #define zig_noreturn _Noreturn
+#define zig_threadlocal thread_local
 #elif __GNUC__
 #define zig_noreturn __attribute__ ((noreturn))
+#define zig_threadlocal __thread
 #elif _MSC_VER
 #define zig_noreturn __declspec(noreturn)
+#define zig_threadlocal __declspec(thread)
 #else
 #define zig_noreturn
-#endif
+#define zig_threadlocal zig_threadlocal_unavailable
-#if defined(__GNUC__)
-#define zig_unreachable() __builtin_unreachable()
-#else
-#define zig_unreachable()
 #endif
 #if __STDC_VERSION__ >= 199901L
 @@ -30,6 +20,20 @@
 #define ZIG_RESTRICT
 #endif
+#if __STDC_VERSION__ >= 199901L
+#include <stdbool.h>
+#else
+#define bool unsigned char
+#define true 1
+#define false 0
+#endif
+#if defined(__GNUC__)
+#define zig_unreachable() __builtin_unreachable()
+#else
+#define zig_unreachable()
+#endif
 #ifdef __cplusplus
 #define ZIG_EXTERN_C extern "C"
 #else

test/stage2/cbe.zig+1-1

...	@@ -517,7 +517,7 @@ pub fn addCases(ctx: *TestContext) !void {	...	@@ -517,7 +517,7 @@ pub fn addCases(ctx: *TestContext) !void {
517	\\}	517	\\}
518	, &.{	518	, &.{
519	":3:21: error: missing struct field: x",	519	":3:21: error: missing struct field: x",
520	":1:15: note: struct 'Point' declared here",	520	":1:15: note: struct 'test_case.Point' declared here",
521	});	521	});
522	case.addError(	522	case.addError(
523	\\const Point = struct { x: i32, y: i32 };	523	\\const Point = struct { x: i32, y: i32 };