From 29e19ace362e7a1910b9f105257f2bce2491e32b Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 24 Jul 2018 10:13:40 -0400 Subject: fix logic for determining whether param requires comptime closes #778 closes #1213 --- src/analyze.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'src/analyze.cpp') diff --git a/src/analyze.cpp b/src/analyze.cpp index 6bbe5f6037..f399ab8305 100644 --- a/src/analyze.cpp +++ b/src/analyze.cpp @@ -1585,10 +1585,6 @@ static TypeTableEntry *analyze_fn_type(CodeGen *g, AstNode *proto_node, Scope *c case TypeTableEntryIdBlock: case TypeTableEntryIdBoundFn: case TypeTableEntryIdMetaType: - add_node_error(g, param_node->data.param_decl.type, - buf_sprintf("parameter of type '%s' must be declared comptime", - buf_ptr(&type_entry->name))); - return g->builtin_types.entry_invalid; case TypeTableEntryIdVoid: case TypeTableEntryIdBool: case TypeTableEntryIdInt: @@ -1603,6 +1599,13 @@ static TypeTableEntry *analyze_fn_type(CodeGen *g, AstNode *proto_node, Scope *c case TypeTableEntryIdUnion: case TypeTableEntryIdFn: case TypeTableEntryIdPromise: + type_ensure_zero_bits_known(g, type_entry); + if (type_requires_comptime(type_entry)) { + add_node_error(g, param_node->data.param_decl.type, + buf_sprintf("parameter of type '%s' must be declared comptime", + buf_ptr(&type_entry->name))); + return g->builtin_types.entry_invalid; + } break; } FnTypeParamInfo *param_info = &fn_type_id.param_info[fn_type_id.next_param_index]; @@ -5019,9 +5022,10 @@ bool type_requires_comptime(TypeTableEntry *type_entry) { } else { return type_requires_comptime(type_entry->data.pointer.child_type); } + case TypeTableEntryIdFn: + return type_entry->data.fn.is_generic; case TypeTableEntryIdEnum: case TypeTableEntryIdErrorSet: - case TypeTableEntryIdFn: case TypeTableEntryIdBool: case TypeTableEntryIdInt: case TypeTableEntryIdFloat: -- cgit v1.2.3 From 2ea08561cf69dabc99722ffc24cb0e4327605506 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 24 Jul 2018 14:20:49 -0400 Subject: self-hosted: function types use table lookup --- src-self-hosted/codegen.zig | 3 +- src-self-hosted/compilation.zig | 69 +++++++- src-self-hosted/ir.zig | 8 +- src-self-hosted/type.zig | 338 +++++++++++++++++++++++++++++++++------- src/analyze.cpp | 8 +- 5 files changed, 356 insertions(+), 70 deletions(-) (limited to 'src/analyze.cpp') diff --git a/src-self-hosted/codegen.zig b/src-self-hosted/codegen.zig index ad3dce061e..88293c845e 100644 --- a/src-self-hosted/codegen.zig +++ b/src-self-hosted/codegen.zig @@ -168,6 +168,7 @@ pub fn renderToLlvmModule(ofile: *ObjectFile, fn_val: *Value.Fn, code: *ir.Code) //} const fn_type = fn_val.base.typ.cast(Type.Fn).?; + const fn_type_normal = &fn_type.key.data.Normal; try addLLVMFnAttr(ofile, llvm_fn, "nounwind"); //add_uwtable_attr(g, fn_table_entry->llvm_value); @@ -209,7 +210,7 @@ pub fn renderToLlvmModule(ofile: *ObjectFile, fn_val: *Value.Fn, code: *ir.Code) // addLLVMArgAttr(fn_table_entry->llvm_value, (unsigned)err_ret_trace_arg_index, "nonnull"); //} - const cur_ret_ptr = if (fn_type.return_type.handleIsPtr()) llvm.GetParam(llvm_fn, 0) else null; + const cur_ret_ptr = if (fn_type_normal.return_type.handleIsPtr()) llvm.GetParam(llvm_fn, 0) else null; // build all basic blocks for (code.basic_block_list.toSlice()) |bb| { diff --git a/src-self-hosted/compilation.zig b/src-self-hosted/compilation.zig index 093aab21da..8d41e2439b 100644 --- a/src-self-hosted/compilation.zig +++ b/src-self-hosted/compilation.zig @@ -220,12 +220,14 @@ pub const Compilation = struct { int_type_table: event.Locked(IntTypeTable), array_type_table: event.Locked(ArrayTypeTable), ptr_type_table: event.Locked(PtrTypeTable), + fn_type_table: event.Locked(FnTypeTable), c_int_types: [CInt.list.len]*Type.Int, const IntTypeTable = std.HashMap(*const Type.Int.Key, *Type.Int, Type.Int.Key.hash, Type.Int.Key.eql); const ArrayTypeTable = std.HashMap(*const Type.Array.Key, *Type.Array, Type.Array.Key.hash, Type.Array.Key.eql); const PtrTypeTable = std.HashMap(*const Type.Pointer.Key, *Type.Pointer, Type.Pointer.Key.hash, Type.Pointer.Key.eql); + const FnTypeTable = std.HashMap(*const Type.Fn.Key, *Type.Fn, Type.Fn.Key.hash, Type.Fn.Key.eql); const TypeTable = std.HashMap([]const u8, *Type, mem.hash_slice_u8, mem.eql_slice_u8); const CompileErrList = std.ArrayList(*Msg); @@ -384,6 +386,7 @@ pub const Compilation = struct { .int_type_table = event.Locked(IntTypeTable).init(loop, IntTypeTable.init(loop.allocator)), .array_type_table = event.Locked(ArrayTypeTable).init(loop, ArrayTypeTable.init(loop.allocator)), .ptr_type_table = event.Locked(PtrTypeTable).init(loop, PtrTypeTable.init(loop.allocator)), + .fn_type_table = event.Locked(FnTypeTable).init(loop, FnTypeTable.init(loop.allocator)), .c_int_types = undefined, .meta_type = undefined, @@ -414,6 +417,7 @@ pub const Compilation = struct { comp.int_type_table.private_data.deinit(); comp.array_type_table.private_data.deinit(); comp.ptr_type_table.private_data.deinit(); + comp.fn_type_table.private_data.deinit(); comp.arena_allocator.deinit(); comp.loop.allocator.destroy(comp); } @@ -1160,10 +1164,47 @@ async fn generateDeclFn(comp: *Compilation, fn_decl: *Decl.Fn) !void { fn_decl.value = Decl.Fn.Val{ .Fn = fn_val }; symbol_name_consumed = true; + // Define local parameter variables + //for (size_t i = 0; i < fn_type_id->param_count; i += 1) { + // FnTypeParamInfo *param_info = &fn_type_id->param_info[i]; + // AstNode *param_decl_node = get_param_decl_node(fn_table_entry, i); + // Buf *param_name; + // bool is_var_args = param_decl_node && param_decl_node->data.param_decl.is_var_args; + // if (param_decl_node && !is_var_args) { + // param_name = param_decl_node->data.param_decl.name; + // } else { + // param_name = buf_sprintf("arg%" ZIG_PRI_usize "", i); + // } + // if (param_name == nullptr) { + // continue; + // } + + // TypeTableEntry *param_type = param_info->type; + // bool is_noalias = param_info->is_noalias; + + // if (is_noalias && get_codegen_ptr_type(param_type) == nullptr) { + // add_node_error(g, param_decl_node, buf_sprintf("noalias on non-pointer parameter")); + // } + + // VariableTableEntry *var = add_variable(g, param_decl_node, fn_table_entry->child_scope, + // param_name, true, create_const_runtime(param_type), nullptr); + // var->src_arg_index = i; + // fn_table_entry->child_scope = var->child_scope; + // var->shadowable = var->shadowable || is_var_args; + + // if (type_has_bits(param_type)) { + // fn_table_entry->variable_list.append(var); + // } + + // if (fn_type->data.fn.gen_param_info) { + // var->gen_arg_index = fn_type->data.fn.gen_param_info[i].gen_index; + // } + //} + const analyzed_code = try await (async comp.genAndAnalyzeCode( &fndef_scope.base, body_node, - fn_type.return_type, + fn_type.key.data.Normal.return_type, ) catch unreachable); errdefer analyzed_code.destroy(comp.gpa()); @@ -1199,14 +1240,13 @@ async fn analyzeFnType(comp: *Compilation, scope: *Scope, fn_proto: *ast.Node.Fn var params = ArrayList(Type.Fn.Param).init(comp.gpa()); var params_consumed = false; - defer if (params_consumed) { + defer if (!params_consumed) { for (params.toSliceConst()) |param| { param.typ.base.deref(comp); } params.deinit(); }; - const is_var_args = false; { var it = fn_proto.params.iterator(0); while (it.next()) |param_node_ptr| { @@ -1219,8 +1259,29 @@ async fn analyzeFnType(comp: *Compilation, scope: *Scope, fn_proto: *ast.Node.Fn }); } } - const fn_type = try Type.Fn.create(comp, return_type, params.toOwnedSlice(), is_var_args); + + const key = Type.Fn.Key{ + .alignment = null, + .data = Type.Fn.Key.Data{ + .Normal = Type.Fn.Normal{ + .return_type = return_type, + .params = params.toOwnedSlice(), + .is_var_args = false, // TODO + .cc = Type.Fn.CallingConvention.Auto, // TODO + }, + }, + }; params_consumed = true; + var key_consumed = false; + defer if (!key_consumed) { + for (key.data.Normal.params) |param| { + param.typ.base.deref(comp); + } + comp.gpa().free(key.data.Normal.params); + }; + + const fn_type = try await (async Type.Fn.get(comp, key) catch unreachable); + key_consumed = true; errdefer fn_type.base.base.deref(comp); return fn_type; diff --git a/src-self-hosted/ir.zig b/src-self-hosted/ir.zig index c34f06753d..45355bbf2c 100644 --- a/src-self-hosted/ir.zig +++ b/src-self-hosted/ir.zig @@ -281,11 +281,13 @@ pub const Inst = struct { return error.SemanticAnalysisFailed; }; - if (fn_type.params.len != self.params.args.len) { + const fn_type_param_count = fn_type.paramCount(); + + if (fn_type_param_count != self.params.args.len) { try ira.addCompileError( self.base.span, "expected {} arguments, found {}", - fn_type.params.len, + fn_type_param_count, self.params.args.len, ); return error.SemanticAnalysisFailed; @@ -299,7 +301,7 @@ pub const Inst = struct { .fn_ref = fn_ref, .args = args, }); - new_inst.val = IrVal{ .KnownType = fn_type.return_type }; + new_inst.val = IrVal{ .KnownType = fn_type.key.data.Normal.return_type }; return new_inst; } diff --git a/src-self-hosted/type.zig b/src-self-hosted/type.zig index 217c1d50a7..3b57260447 100644 --- a/src-self-hosted/type.zig +++ b/src-self-hosted/type.zig @@ -221,57 +221,267 @@ pub const Type = struct { pub const Fn = struct { base: Type, - return_type: *Type, - params: []Param, - is_var_args: bool, + key: Key, + garbage_node: std.atomic.Stack(*Fn).Node, + + pub const Key = struct { + data: Data, + alignment: ?u32, + + pub const Data = union(enum) { + Generic: Generic, + Normal: Normal, + }; + + pub fn hash(self: *const Key) u32 { + var result: u32 = 0; + result +%= hashAny(self.alignment, 0); + switch (self.data) { + Data.Generic => |generic| { + result +%= hashAny(generic.param_count, 1); + switch (generic.cc) { + CallingConvention.Async => |allocator_type| result +%= hashAny(allocator_type, 2), + else => result +%= hashAny(CallingConvention(generic.cc), 3), + } + }, + Data.Normal => |normal| { + result +%= hashAny(normal.return_type, 4); + result +%= hashAny(normal.is_var_args, 5); + result +%= hashAny(normal.cc, 6); + for (normal.params) |param| { + result +%= hashAny(param.is_noalias, 7); + result +%= hashAny(param.typ, 8); + } + }, + } + return result; + } + + pub fn eql(self: *const Key, other: *const Key) bool { + if ((self.alignment == null) != (other.alignment == null)) return false; + if (self.alignment) |self_align| { + if (self_align != other.alignment.?) return false; + } + if (@TagType(Data)(self.data) != @TagType(Data)(other.data)) return false; + switch (self.data) { + Data.Generic => |*self_generic| { + const other_generic = &other.data.Generic; + if (self_generic.param_count != other_generic.param_count) return false; + if (CallingConvention(self_generic.cc) != CallingConvention(other_generic.cc)) return false; + switch (self_generic.cc) { + CallingConvention.Async => |self_allocator_type| { + const other_allocator_type = other_generic.cc.Async; + if (self_allocator_type != other_allocator_type) return false; + }, + else => {}, + } + }, + Data.Normal => |*self_normal| { + const other_normal = &other.data.Normal; + if (self_normal.cc != other_normal.cc) return false; + if (self_normal.is_var_args != other_normal.is_var_args) return false; + if (self_normal.return_type != other_normal.return_type) return false; + for (self_normal.params) |*self_param, i| { + const other_param = &other_normal.params[i]; + if (self_param.is_noalias != other_param.is_noalias) return false; + if (self_param.typ != other_param.typ) return false; + } + }, + } + return true; + } + + pub fn deref(key: Key, comp: *Compilation) void { + switch (key.data) { + Key.Data.Generic => |generic| { + switch (generic.cc) { + CallingConvention.Async => |allocator_type| allocator_type.base.deref(comp), + else => {}, + } + }, + Key.Data.Normal => |normal| { + normal.return_type.base.deref(comp); + for (normal.params) |param| { + param.typ.base.deref(comp); + } + }, + } + } + + pub fn ref(key: Key) void { + switch (key.data) { + Key.Data.Generic => |generic| { + switch (generic.cc) { + CallingConvention.Async => |allocator_type| allocator_type.base.ref(), + else => {}, + } + }, + Key.Data.Normal => |normal| { + normal.return_type.base.ref(); + for (normal.params) |param| { + param.typ.base.ref(); + } + }, + } + } + }; + + pub const Normal = struct { + params: []Param, + return_type: *Type, + is_var_args: bool, + cc: CallingConvention, + }; + + pub const Generic = struct { + param_count: usize, + cc: CC, + + pub const CC = union(CallingConvention) { + Auto, + C, + Cold, + Naked, + Stdcall, + Async: *Type, // allocator type + }; + }; + + pub const CallingConvention = enum { + Auto, + C, + Cold, + Naked, + Stdcall, + Async, + }; pub const Param = struct { is_noalias: bool, typ: *Type, }; - pub fn create(comp: *Compilation, return_type: *Type, params: []Param, is_var_args: bool) !*Fn { - const result = try comp.gpa().create(Fn{ + fn ccFnTypeStr(cc: CallingConvention) []const u8 { + return switch (cc) { + CallingConvention.Auto => "", + CallingConvention.C => "extern ", + CallingConvention.Cold => "coldcc ", + CallingConvention.Naked => "nakedcc ", + CallingConvention.Stdcall => "stdcallcc ", + CallingConvention.Async => unreachable, + }; + } + + pub fn paramCount(self: *Fn) usize { + return switch (self.key.data) { + Key.Data.Generic => |generic| generic.param_count, + Key.Data.Normal => |normal| normal.params.len, + }; + } + + /// takes ownership of key.Normal.params on success + pub async fn get(comp: *Compilation, key: Key) !*Fn { + { + const held = await (async comp.fn_type_table.acquire() catch unreachable); + defer held.release(); + + if (held.value.get(&key)) |entry| { + entry.value.base.base.ref(); + return entry.value; + } + } + + key.ref(); + errdefer key.deref(comp); + + const self = try comp.gpa().create(Fn{ .base = undefined, - .return_type = return_type, - .params = params, - .is_var_args = is_var_args, + .key = key, + .garbage_node = undefined, }); - errdefer comp.gpa().destroy(result); + errdefer comp.gpa().destroy(self); - result.base.init(comp, Id.Fn, "TODO fn type name"); + var name_buf = try std.Buffer.initSize(comp.gpa(), 0); + defer name_buf.deinit(); + + const name_stream = &std.io.BufferOutStream.init(&name_buf).stream; + + switch (key.data) { + Key.Data.Generic => |generic| { + switch (generic.cc) { + CallingConvention.Async => |async_allocator_type| { + try name_stream.print("async<{}> ", async_allocator_type.name); + }, + else => { + const cc_str = ccFnTypeStr(generic.cc); + try name_stream.write(cc_str); + }, + } + try name_stream.write("fn("); + var param_i: usize = 0; + while (param_i < generic.param_count) : (param_i += 1) { + const arg = if (param_i == 0) "var" else ", var"; + try name_stream.write(arg); + } + try name_stream.write(")"); + if (key.alignment) |alignment| { + try name_stream.print(" align<{}>", alignment); + } + try name_stream.write(" var"); + }, + Key.Data.Normal => |normal| { + const cc_str = ccFnTypeStr(normal.cc); + try name_stream.print("{}fn(", cc_str); + for (normal.params) |param, i| { + if (i != 0) try name_stream.write(", "); + if (param.is_noalias) try name_stream.write("noalias "); + try name_stream.write(param.typ.name); + } + if (normal.is_var_args) { + if (normal.params.len != 0) try name_stream.write(", "); + try name_stream.write("..."); + } + try name_stream.write(")"); + if (key.alignment) |alignment| { + try name_stream.print(" align<{}>", alignment); + } + try name_stream.print(" {}", normal.return_type.name); + }, + } + + self.base.init(comp, Id.Fn, name_buf.toOwnedSlice()); - result.return_type.base.ref(); - for (result.params) |param| { - param.typ.base.ref(); + { + const held = await (async comp.fn_type_table.acquire() catch unreachable); + defer held.release(); + + _ = try held.value.put(&self.key, self); } - return result; + return self; } pub fn destroy(self: *Fn, comp: *Compilation) void { - self.return_type.base.deref(comp); - for (self.params) |param| { - param.typ.base.deref(comp); - } + self.key.deref(comp); comp.gpa().destroy(self); } pub fn getLlvmType(self: *Fn, allocator: *Allocator, llvm_context: llvm.ContextRef) !llvm.TypeRef { - const llvm_return_type = switch (self.return_type.id) { + const normal = &self.key.data.Normal; + const llvm_return_type = switch (normal.return_type.id) { Type.Id.Void => llvm.VoidTypeInContext(llvm_context) orelse return error.OutOfMemory, - else => try self.return_type.getLlvmType(allocator, llvm_context), + else => try normal.return_type.getLlvmType(allocator, llvm_context), }; - const llvm_param_types = try allocator.alloc(llvm.TypeRef, self.params.len); + const llvm_param_types = try allocator.alloc(llvm.TypeRef, normal.params.len); defer allocator.free(llvm_param_types); for (llvm_param_types) |*llvm_param_type, i| { - llvm_param_type.* = try self.params[i].typ.getLlvmType(allocator, llvm_context); + llvm_param_type.* = try normal.params[i].typ.getLlvmType(allocator, llvm_context); } return llvm.FunctionType( llvm_return_type, llvm_param_types.ptr, @intCast(c_uint, llvm_param_types.len), - @boolToInt(self.is_var_args), + @boolToInt(normal.is_var_args), ) orelse error.OutOfMemory; } }; @@ -347,8 +557,10 @@ pub const Type = struct { is_signed: bool, pub fn hash(self: *const Key) u32 { - const rands = [2]u32{ 0xa4ba6498, 0x75fc5af7 }; - return rands[@boolToInt(self.is_signed)] *% self.bit_count; + var result: u32 = 0; + result +%= hashAny(self.is_signed, 0); + result +%= hashAny(self.bit_count, 1); + return result; } pub fn eql(self: *const Key, other: *const Key) bool { @@ -443,15 +655,16 @@ pub const Type = struct { alignment: Align, pub fn hash(self: *const Key) u32 { - const align_hash = switch (self.alignment) { + var result: u32 = 0; + result +%= switch (self.alignment) { Align.Abi => 0xf201c090, - Align.Override => |x| x, + Align.Override => |x| hashAny(x, 0), }; - return hash_usize(@ptrToInt(self.child_type)) *% - hash_enum(self.mut) *% - hash_enum(self.vol) *% - hash_enum(self.size) *% - align_hash; + result +%= hashAny(self.child_type, 1); + result +%= hashAny(self.mut, 2); + result +%= hashAny(self.vol, 3); + result +%= hashAny(self.size, 4); + return result; } pub fn eql(self: *const Key, other: *const Key) bool { @@ -605,7 +818,10 @@ pub const Type = struct { len: usize, pub fn hash(self: *const Key) u32 { - return hash_usize(@ptrToInt(self.elem_type)) *% hash_usize(self.len); + var result: u32 = 0; + result +%= hashAny(self.elem_type, 0); + result +%= hashAny(self.len, 1); + return result; } pub fn eql(self: *const Key, other: *const Key) bool { @@ -818,27 +1034,37 @@ pub const Type = struct { }; }; -fn hash_usize(x: usize) u32 { - return switch (@sizeOf(usize)) { - 4 => x, - 8 => @truncate(u32, x *% 0xad44ee2d8e3fc13d), - else => @compileError("implement this hash function"), - }; -} - -fn hash_enum(x: var) u32 { - const rands = []u32{ - 0x85ebf64f, - 0x3fcb3211, - 0x240a4e8e, - 0x40bb0e3c, - 0x78be45af, - 0x1ca98e37, - 0xec56053a, - 0x906adc48, - 0xd4fe9763, - 0x54c80dac, - }; - comptime assert(@memberCount(@typeOf(x)) < rands.len); - return rands[@enumToInt(x)]; +fn hashAny(x: var, comptime seed: u64) u32 { + switch (@typeInfo(@typeOf(x))) { + builtin.TypeId.Int => |info| { + comptime var rng = comptime std.rand.DefaultPrng.init(seed); + const unsigned_x = @bitCast(@IntType(false, info.bits), x); + if (info.bits <= 32) { + return u32(unsigned_x) *% comptime rng.random.scalar(u32); + } else { + return @truncate(u32, unsigned_x *% comptime rng.random.scalar(@typeOf(unsigned_x))); + } + }, + builtin.TypeId.Pointer => |info| { + switch (info.size) { + builtin.TypeInfo.Pointer.Size.One => return hashAny(@ptrToInt(x), seed), + builtin.TypeInfo.Pointer.Size.Many => @compileError("implement hash function"), + builtin.TypeInfo.Pointer.Size.Slice => @compileError("implement hash function"), + } + }, + builtin.TypeId.Enum => return hashAny(@enumToInt(x), seed), + builtin.TypeId.Bool => { + comptime var rng = comptime std.rand.DefaultPrng.init(seed); + const vals = comptime [2]u32{ rng.random.scalar(u32), rng.random.scalar(u32) }; + return vals[@boolToInt(x)]; + }, + builtin.TypeId.Optional => { + if (x) |non_opt| { + return hashAny(non_opt, seed); + } else { + return hashAny(u32(1), seed); + } + }, + else => @compileError("implement hash function for " ++ @typeName(@typeOf(x))), + } } diff --git a/src/analyze.cpp b/src/analyze.cpp index f399ab8305..a4bfff78c3 100644 --- a/src/analyze.cpp +++ b/src/analyze.cpp @@ -3941,7 +3941,7 @@ AstNode *get_param_decl_node(FnTableEntry *fn_entry, size_t index) { return nullptr; } -static void define_local_param_variables(CodeGen *g, FnTableEntry *fn_table_entry, VariableTableEntry **arg_vars) { +static void define_local_param_variables(CodeGen *g, FnTableEntry *fn_table_entry) { TypeTableEntry *fn_type = fn_table_entry->type_entry; assert(!fn_type->data.fn.is_generic); FnTypeId *fn_type_id = &fn_type->data.fn.fn_type_id; @@ -3979,10 +3979,6 @@ static void define_local_param_variables(CodeGen *g, FnTableEntry *fn_table_entr if (fn_type->data.fn.gen_param_info) { var->gen_arg_index = fn_type->data.fn.gen_param_info[i].gen_index; } - - if (arg_vars) { - arg_vars[i] = var; - } } } @@ -4082,7 +4078,7 @@ static void analyze_fn_body(CodeGen *g, FnTableEntry *fn_table_entry) { if (!fn_table_entry->child_scope) fn_table_entry->child_scope = &fn_table_entry->fndef_scope->base; - define_local_param_variables(g, fn_table_entry, nullptr); + define_local_param_variables(g, fn_table_entry); TypeTableEntry *fn_type = fn_table_entry->type_entry; assert(!fn_type->data.fn.is_generic); -- cgit v1.2.3 From 2cbad364c1d23b64ae064f8547590c133b4f070a Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 26 Jul 2018 18:29:07 -0400 Subject: add compile error for ignoring return value of while loop bodies closes #1049 --- src/analyze.cpp | 2 +- src/ir.cpp | 12 +++++++++--- src/ir_print.cpp | 4 ++++ test/compile_errors.zig | 22 ++++++++++++++++++++++ 4 files changed, 36 insertions(+), 4 deletions(-) (limited to 'src/analyze.cpp') diff --git a/src/analyze.cpp b/src/analyze.cpp index a4bfff78c3..aadee29fc8 100644 --- a/src/analyze.cpp +++ b/src/analyze.cpp @@ -4056,7 +4056,7 @@ void analyze_fn_ir(CodeGen *g, FnTableEntry *fn_table_entry, AstNode *return_typ } if (g->verbose_ir) { - fprintf(stderr, "{ // (analyzed)\n"); + fprintf(stderr, "fn %s() { // (analyzed)\n", buf_ptr(&fn_table_entry->symbol_name)); ir_print(g, stderr, &fn_table_entry->analyzed_executable, 4); fprintf(stderr, "}\n"); } diff --git a/src/ir.cpp b/src/ir.cpp index e40c129953..a6007852e0 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -5251,8 +5251,10 @@ static IrInstruction *ir_gen_while_expr(IrBuilder *irb, Scope *scope, AstNode *n if (body_result == irb->codegen->invalid_instruction) return body_result; - if (!instr_is_unreachable(body_result)) + if (!instr_is_unreachable(body_result)) { + ir_mark_gen(ir_build_check_statement_is_void(irb, payload_scope, node->data.while_expr.body, body_result)); ir_mark_gen(ir_build_br(irb, payload_scope, node, continue_block, is_comptime)); + } if (continue_expr_node) { ir_set_cursor_at_end_and_append_block(irb, continue_block); @@ -5331,8 +5333,10 @@ static IrInstruction *ir_gen_while_expr(IrBuilder *irb, Scope *scope, AstNode *n if (body_result == irb->codegen->invalid_instruction) return body_result; - if (!instr_is_unreachable(body_result)) + if (!instr_is_unreachable(body_result)) { + ir_mark_gen(ir_build_check_statement_is_void(irb, child_scope, node->data.while_expr.body, body_result)); ir_mark_gen(ir_build_br(irb, child_scope, node, continue_block, is_comptime)); + } if (continue_expr_node) { ir_set_cursor_at_end_and_append_block(irb, continue_block); @@ -5392,8 +5396,10 @@ static IrInstruction *ir_gen_while_expr(IrBuilder *irb, Scope *scope, AstNode *n if (body_result == irb->codegen->invalid_instruction) return body_result; - if (!instr_is_unreachable(body_result)) + if (!instr_is_unreachable(body_result)) { + ir_mark_gen(ir_build_check_statement_is_void(irb, scope, node->data.while_expr.body, body_result)); ir_mark_gen(ir_build_br(irb, scope, node, continue_block, is_comptime)); + } if (continue_expr_node) { ir_set_cursor_at_end_and_append_block(irb, continue_block); diff --git a/src/ir_print.cpp b/src/ir_print.cpp index 6182958d0a..127afa94a5 100644 --- a/src/ir_print.cpp +++ b/src/ir_print.cpp @@ -45,6 +45,10 @@ static void ir_print_var_instruction(IrPrint *irp, IrInstruction *instruction) { } static void ir_print_other_instruction(IrPrint *irp, IrInstruction *instruction) { + if (instruction == nullptr) { + fprintf(irp->f, "(null)"); + return; + } if (instruction->value.special != ConstValSpecialRuntime) { ir_print_const_value(irp, &instruction->value); } else { diff --git a/test/compile_errors.zig b/test/compile_errors.zig index 83bf715f78..2c4c9208eb 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -1,6 +1,28 @@ const tests = @import("tests.zig"); pub fn addCases(cases: *tests.CompileErrorContext) void { + cases.add( + "while loop body expression ignored", + \\fn returns() usize { + \\ return 2; + \\} + \\export fn f1() void { + \\ while (true) returns(); + \\} + \\export fn f2() void { + \\ var x: ?i32 = null; + \\ while (x) |_| returns(); + \\} + \\export fn f3() void { + \\ var x: error!i32 = error.Bad; + \\ while (x) |_| returns() else |_| unreachable; + \\} + , + ".tmp_source.zig:5:25: error: expression value is ignored", + ".tmp_source.zig:9:26: error: expression value is ignored", + ".tmp_source.zig:13:26: error: expression value is ignored", + ); + cases.add( "missing parameter name of generic function", \\fn dump(var) void {} -- cgit v1.2.3 From b3f4182ca1756ccf84fe5bbc88594a91ead617b5 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 26 Jul 2018 22:26:00 -0400 Subject: coroutines have 3 more bits of atomic state --- src/all_types.hpp | 2 +- src/analyze.cpp | 13 ++++++--- src/ir.cpp | 80 ++++++++++++++++++++++++++++++++++++++----------------- 3 files changed, 66 insertions(+), 29 deletions(-) (limited to 'src/analyze.cpp') diff --git a/src/all_types.hpp b/src/all_types.hpp index bcd6a04cc3..70ea629c59 100644 --- a/src/all_types.hpp +++ b/src/all_types.hpp @@ -3245,7 +3245,7 @@ static const size_t stack_trace_ptr_count = 30; #define RESULT_FIELD_NAME "result" #define ASYNC_ALLOC_FIELD_NAME "allocFn" #define ASYNC_FREE_FIELD_NAME "freeFn" -#define AWAITER_HANDLE_FIELD_NAME "awaiter_handle" +#define ATOMIC_STATE_FIELD_NAME "atomic_state" // these point to data belonging to the awaiter #define ERR_RET_TRACE_PTR_FIELD_NAME "err_ret_trace_ptr" #define RESULT_PTR_FIELD_NAME "result_ptr" diff --git a/src/analyze.cpp b/src/analyze.cpp index aadee29fc8..74d59f966a 100644 --- a/src/analyze.cpp +++ b/src/analyze.cpp @@ -519,11 +519,11 @@ TypeTableEntry *get_promise_frame_type(CodeGen *g, TypeTableEntry *return_type) return return_type->promise_frame_parent; } - TypeTableEntry *awaiter_handle_type = get_optional_type(g, g->builtin_types.entry_promise); + TypeTableEntry *atomic_state_type = g->builtin_types.entry_usize; TypeTableEntry *result_ptr_type = get_pointer_to_type(g, return_type, false); ZigList field_names = {}; - field_names.append(AWAITER_HANDLE_FIELD_NAME); + field_names.append(ATOMIC_STATE_FIELD_NAME); field_names.append(RESULT_FIELD_NAME); field_names.append(RESULT_PTR_FIELD_NAME); if (g->have_err_ret_tracing) { @@ -533,7 +533,7 @@ TypeTableEntry *get_promise_frame_type(CodeGen *g, TypeTableEntry *return_type) } ZigList field_types = {}; - field_types.append(awaiter_handle_type); + field_types.append(atomic_state_type); field_types.append(return_type); field_types.append(result_ptr_type); if (g->have_err_ret_tracing) { @@ -6228,7 +6228,12 @@ uint32_t get_abi_alignment(CodeGen *g, TypeTableEntry *type_entry) { } else if (type_entry->id == TypeTableEntryIdOpaque) { return 1; } else { - return LLVMABIAlignmentOfType(g->target_data_ref, type_entry->type_ref); + uint32_t llvm_alignment = LLVMABIAlignmentOfType(g->target_data_ref, type_entry->type_ref); + // promises have at least alignment 8 so that we can have 3 extra bits when doing atomicrmw + if (type_entry->id == TypeTableEntryIdPromise && llvm_alignment < 8) { + return 8; + } + return llvm_alignment; } } diff --git a/src/ir.cpp b/src/ir.cpp index a6007852e0..5466e64e55 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -3097,19 +3097,47 @@ static IrInstruction *ir_gen_async_return(IrBuilder *irb, Scope *scope, AstNode return return_inst; } + IrBasicBlock *canceled_block = ir_create_basic_block(irb, scope, "Canceled"); + IrBasicBlock *not_canceled_block = ir_create_basic_block(irb, scope, "NotCanceled"); + IrBasicBlock *suspended_block = ir_create_basic_block(irb, scope, "Suspended"); + IrBasicBlock *not_suspended_block = ir_create_basic_block(irb, scope, "NotSuspended"); + ir_build_store_ptr(irb, scope, node, irb->exec->coro_result_field_ptr, return_value); - IrInstruction *promise_type_val = ir_build_const_type(irb, scope, node, - get_optional_type(irb->codegen, irb->codegen->builtin_types.entry_promise)); - // TODO replace replacement_value with @intToPtr(?promise, 0x1) when it doesn't crash zig - IrInstruction *replacement_value = irb->exec->coro_handle; - IrInstruction *maybe_await_handle = ir_build_atomic_rmw(irb, scope, node, - promise_type_val, irb->exec->coro_awaiter_field_ptr, nullptr, replacement_value, nullptr, - AtomicRmwOp_xchg, AtomicOrderSeqCst); - ir_build_store_ptr(irb, scope, node, irb->exec->await_handle_var_ptr, maybe_await_handle); - IrInstruction *is_non_null = ir_build_test_nonnull(irb, scope, node, maybe_await_handle); + IrInstruction *usize_type_val = ir_build_const_type(irb, scope, node, irb->codegen->builtin_types.entry_usize); + IrInstruction *replacement_value = ir_build_const_usize(irb, scope, node, 0xa); // 0b1010 + IrInstruction *prev_atomic_value = ir_build_atomic_rmw(irb, scope, node, + usize_type_val, irb->exec->coro_awaiter_field_ptr, nullptr, replacement_value, nullptr, + AtomicRmwOp_or, AtomicOrderSeqCst); + + IrInstruction *zero = ir_build_const_usize(irb, scope, node, 0); IrInstruction *is_comptime = ir_build_const_bool(irb, scope, node, false); - return ir_build_cond_br(irb, scope, node, is_non_null, irb->exec->coro_normal_final, irb->exec->coro_early_final, - is_comptime); + IrInstruction *is_canceled_mask = ir_build_const_usize(irb, scope, node, 0x1); // 0b001 + IrInstruction *is_canceled_value = ir_build_bin_op(irb, scope, node, IrBinOpBinAnd, prev_atomic_value, is_canceled_mask, false); + IrInstruction *is_canceled_bool = ir_build_bin_op(irb, scope, node, IrBinOpCmpNotEq, is_canceled_value, zero, false); + ir_build_cond_br(irb, scope, node, is_canceled_bool, canceled_block, not_canceled_block, is_comptime); + + ir_set_cursor_at_end_and_append_block(irb, canceled_block); + ir_mark_gen(ir_build_br(irb, scope, node, irb->exec->coro_final_cleanup_block, is_comptime)); + + ir_set_cursor_at_end_and_append_block(irb, not_canceled_block); + IrInstruction *inverted_ptr_mask = ir_build_const_usize(irb, scope, node, 0x7); // 0b111 + IrInstruction *is_suspended_value = ir_build_bin_op(irb, scope, node, IrBinOpBinAnd, prev_atomic_value, inverted_ptr_mask, false); + IrInstruction *is_suspended_bool = ir_build_bin_op(irb, scope, node, IrBinOpCmpNotEq, is_suspended_value, zero, false); + ir_build_cond_br(irb, scope, node, is_suspended_bool, suspended_block, not_suspended_block, is_comptime); + + ir_set_cursor_at_end_and_append_block(irb, suspended_block); + ir_build_unreachable(irb, scope, node); + + ir_set_cursor_at_end_and_append_block(irb, not_suspended_block); + IrInstruction *ptr_mask = ir_build_un_op(irb, scope, node, IrUnOpBinNot, inverted_ptr_mask); // 0b111...000 + IrInstruction *await_handle_addr = ir_build_bin_op(irb, scope, node, IrBinOpBinAnd, prev_atomic_value, ptr_mask, false); + IrInstruction *promise_type_val = ir_build_const_type(irb, scope, node, irb->codegen->builtin_types.entry_promise); + // if we ever add null checking safety to the ptrtoint instruction, it needs to be disabled here + IrInstruction *await_handle = ir_build_int_to_ptr(irb, scope, node, promise_type_val, await_handle_addr); + ir_build_store_ptr(irb, scope, node, irb->exec->await_handle_var_ptr, await_handle); + IrInstruction *is_non_null = ir_build_bin_op(irb, scope, node, IrBinOpCmpNotEq, await_handle_addr, zero, false); + return ir_build_cond_br(irb, scope, node, is_non_null, irb->exec->coro_normal_final, + irb->exec->coro_early_final, is_comptime); // the above blocks are rendered by ir_gen after the rest of codegen } @@ -6708,9 +6736,9 @@ static IrInstruction *ir_gen_await_expr(IrBuilder *irb, Scope *parent_scope, Ast ir_build_store_ptr(irb, parent_scope, node, err_ret_trace_ptr_field_ptr, err_ret_trace_ptr); } - Buf *awaiter_handle_field_name = buf_create_from_str(AWAITER_HANDLE_FIELD_NAME); - IrInstruction *awaiter_field_ptr = ir_build_field_ptr(irb, parent_scope, node, coro_promise_ptr, - awaiter_handle_field_name); + Buf *atomic_state_field_name = buf_create_from_str(ATOMIC_STATE_FIELD_NAME); + IrInstruction *atomic_state_ptr = ir_build_field_ptr(irb, parent_scope, node, coro_promise_ptr, + atomic_state_field_name); IrInstruction *const_bool_false = ir_build_const_bool(irb, parent_scope, node, false); VariableTableEntry *result_var = ir_create_var(irb, node, parent_scope, nullptr, @@ -6723,12 +6751,16 @@ static IrInstruction *ir_gen_await_expr(IrBuilder *irb, Scope *parent_scope, Ast IrInstruction *my_result_var_ptr = ir_build_var_ptr(irb, parent_scope, node, result_var); ir_build_store_ptr(irb, parent_scope, node, result_ptr_field_ptr, my_result_var_ptr); IrInstruction *save_token = ir_build_coro_save(irb, parent_scope, node, irb->exec->coro_handle); - IrInstruction *promise_type_val = ir_build_const_type(irb, parent_scope, node, - get_optional_type(irb->codegen, irb->codegen->builtin_types.entry_promise)); - IrInstruction *maybe_await_handle = ir_build_atomic_rmw(irb, parent_scope, node, - promise_type_val, awaiter_field_ptr, nullptr, irb->exec->coro_handle, nullptr, - AtomicRmwOp_xchg, AtomicOrderSeqCst); - IrInstruction *is_non_null = ir_build_test_nonnull(irb, parent_scope, node, maybe_await_handle); + IrInstruction *usize_type_val = ir_build_const_type(irb, parent_scope, node, irb->codegen->builtin_types.entry_usize); + IrInstruction *coro_handle_addr = ir_build_ptr_to_int(irb, parent_scope, node, irb->exec->coro_handle); + IrInstruction *prev_atomic_value = ir_build_atomic_rmw(irb, parent_scope, node, + usize_type_val, atomic_state_ptr, nullptr, coro_handle_addr, nullptr, + AtomicRmwOp_or, AtomicOrderSeqCst); + IrInstruction *zero = ir_build_const_usize(irb, parent_scope, node, 0); + IrInstruction *inverted_ptr_mask = ir_build_const_usize(irb, parent_scope, node, 0x7); // 0b111 + IrInstruction *ptr_mask = ir_build_un_op(irb, parent_scope, node, IrUnOpBinNot, inverted_ptr_mask); // 0b111...000 + IrInstruction *await_handle_addr = ir_build_bin_op(irb, parent_scope, node, IrBinOpBinAnd, prev_atomic_value, ptr_mask, false); + IrInstruction *is_non_null = ir_build_bin_op(irb, parent_scope, node, IrBinOpCmpNotEq, await_handle_addr, zero, false); IrBasicBlock *yes_suspend_block = ir_create_basic_block(irb, parent_scope, "YesSuspend"); IrBasicBlock *no_suspend_block = ir_create_basic_block(irb, parent_scope, "NoSuspend"); IrBasicBlock *merge_block = ir_create_basic_block(irb, parent_scope, "MergeSuspend"); @@ -7087,10 +7119,11 @@ bool ir_gen(CodeGen *codegen, AstNode *node, Scope *scope, IrExecutable *ir_exec IrInstruction *coro_mem_ptr = ir_build_ptr_cast(irb, coro_scope, node, u8_ptr_type, maybe_coro_mem_ptr); irb->exec->coro_handle = ir_build_coro_begin(irb, coro_scope, node, coro_id, coro_mem_ptr); - Buf *awaiter_handle_field_name = buf_create_from_str(AWAITER_HANDLE_FIELD_NAME); + Buf *atomic_state_field_name = buf_create_from_str(ATOMIC_STATE_FIELD_NAME); irb->exec->coro_awaiter_field_ptr = ir_build_field_ptr(irb, scope, node, coro_promise_ptr, - awaiter_handle_field_name); - ir_build_store_ptr(irb, scope, node, irb->exec->coro_awaiter_field_ptr, null_value); + atomic_state_field_name); + IrInstruction *zero = ir_build_const_usize(irb, scope, node, 0); + ir_build_store_ptr(irb, scope, node, irb->exec->coro_awaiter_field_ptr, zero); Buf *result_field_name = buf_create_from_str(RESULT_FIELD_NAME); irb->exec->coro_result_field_ptr = ir_build_field_ptr(irb, scope, node, coro_promise_ptr, result_field_name); result_ptr_field_name = buf_create_from_str(RESULT_PTR_FIELD_NAME); @@ -7108,7 +7141,6 @@ bool ir_gen(CodeGen *codegen, AstNode *node, Scope *scope, IrExecutable *ir_exec // coordinate with builtin.zig Buf *index_name = buf_create_from_str("index"); IrInstruction *index_ptr = ir_build_field_ptr(irb, scope, node, err_ret_trace_ptr, index_name); - IrInstruction *zero = ir_build_const_usize(irb, scope, node, 0); ir_build_store_ptr(irb, scope, node, index_ptr, zero); Buf *instruction_addresses_name = buf_create_from_str("instruction_addresses"); -- cgit v1.2.3 From 02c5bda704d30e95e6af23804f9a552e9d8ca2d7 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 27 Jul 2018 17:27:03 -0400 Subject: remove ability to break from suspend blocks closes #803 --- doc/langref.html.in | 2 +- src/all_types.hpp | 2 -- src/analyze.cpp | 1 - src/ir.cpp | 17 ++--------------- src/parser.cpp | 25 ++----------------------- 5 files changed, 5 insertions(+), 42 deletions(-) (limited to 'src/analyze.cpp') diff --git a/doc/langref.html.in b/doc/langref.html.in index 60ba09d391..d91fb6e8fb 100644 --- a/doc/langref.html.in +++ b/doc/langref.html.in @@ -7336,7 +7336,7 @@ Defer(body) = ("defer" | "deferror") body IfExpression(body) = "if" "(" Expression ")" body option("else" BlockExpression(body)) -SuspendExpression(body) = option(Symbol ":") "suspend" option(("|" Symbol "|" body)) +SuspendExpression(body) = "suspend" option(("|" Symbol "|" body)) IfErrorExpression(body) = "if" "(" Expression ")" option("|" option("*") Symbol "|") body "else" "|" Symbol "|" BlockExpression(body) diff --git a/src/all_types.hpp b/src/all_types.hpp index 3ac7afe474..2f09e70301 100644 --- a/src/all_types.hpp +++ b/src/all_types.hpp @@ -898,7 +898,6 @@ struct AstNodeAwaitExpr { }; struct AstNodeSuspend { - Buf *name; AstNode *block; AstNode *promise_symbol; }; @@ -1929,7 +1928,6 @@ struct ScopeLoop { struct ScopeSuspend { Scope base; - Buf *name; IrBasicBlock *resume_block; bool reported_err; }; diff --git a/src/analyze.cpp b/src/analyze.cpp index 74d59f966a..03cfa5b67b 100644 --- a/src/analyze.cpp +++ b/src/analyze.cpp @@ -161,7 +161,6 @@ ScopeSuspend *create_suspend_scope(AstNode *node, Scope *parent) { assert(node->type == NodeTypeSuspend); ScopeSuspend *scope = allocate(1); init_scope(&scope->base, ScopeIdSuspend, node, parent); - scope->name = node->data.suspend.name; return scope; } diff --git a/src/ir.cpp b/src/ir.cpp index cd791fb189..799d7e3bc5 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -6186,15 +6186,6 @@ static IrInstruction *ir_gen_return_from_block(IrBuilder *irb, Scope *break_scop return ir_build_br(irb, break_scope, node, dest_block, is_comptime); } -static IrInstruction *ir_gen_break_from_suspend(IrBuilder *irb, Scope *break_scope, AstNode *node, ScopeSuspend *suspend_scope) { - IrInstruction *is_comptime = ir_build_const_bool(irb, break_scope, node, false); - - IrBasicBlock *dest_block = suspend_scope->resume_block; - ir_gen_defers_for_block(irb, break_scope, dest_block->scope, false); - - return ir_build_br(irb, break_scope, node, dest_block, is_comptime); -} - static IrInstruction *ir_gen_break(IrBuilder *irb, Scope *break_scope, AstNode *node) { assert(node->type == NodeTypeBreak); @@ -6235,12 +6226,8 @@ static IrInstruction *ir_gen_break(IrBuilder *irb, Scope *break_scope, AstNode * return ir_gen_return_from_block(irb, break_scope, node, this_block_scope); } } else if (search_scope->id == ScopeIdSuspend) { - ScopeSuspend *this_suspend_scope = (ScopeSuspend *)search_scope; - if (node->data.break_expr.name != nullptr && - (this_suspend_scope->name != nullptr && buf_eql_buf(node->data.break_expr.name, this_suspend_scope->name))) - { - return ir_gen_break_from_suspend(irb, break_scope, node, this_suspend_scope); - } + add_node_error(irb->codegen, node, buf_sprintf("cannot break out of suspend block")); + return irb->codegen->invalid_instruction; } search_scope = search_scope->parent; } diff --git a/src/parser.cpp b/src/parser.cpp index adb1633f5d..a93d8de830 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -648,30 +648,12 @@ static AstNode *ast_parse_asm_expr(ParseContext *pc, size_t *token_index, bool m } /* -SuspendExpression(body) = option(Symbol ":") "suspend" option(("|" Symbol "|" body)) +SuspendExpression(body) = "suspend" option(("|" Symbol "|" body)) */ static AstNode *ast_parse_suspend_block(ParseContext *pc, size_t *token_index, bool mandatory) { size_t orig_token_index = *token_index; - Token *name_token = nullptr; - Token *token = &pc->tokens->at(*token_index); - if (token->id == TokenIdSymbol) { - *token_index += 1; - Token *colon_token = &pc->tokens->at(*token_index); - if (colon_token->id == TokenIdColon) { - *token_index += 1; - name_token = token; - token = &pc->tokens->at(*token_index); - } else if (mandatory) { - ast_expect_token(pc, colon_token, TokenIdColon); - zig_unreachable(); - } else { - *token_index = orig_token_index; - return nullptr; - } - } - - Token *suspend_token = token; + Token *suspend_token = &pc->tokens->at(*token_index); if (suspend_token->id == TokenIdKeywordSuspend) { *token_index += 1; } else if (mandatory) { @@ -693,9 +675,6 @@ static AstNode *ast_parse_suspend_block(ParseContext *pc, size_t *token_index, b } AstNode *node = ast_create_node(pc, NodeTypeSuspend, suspend_token); - if (name_token != nullptr) { - node->data.suspend.name = token_buf(name_token); - } node->data.suspend.promise_symbol = ast_parse_symbol(pc, token_index); ast_eat_token(pc, token_index, TokenIdBinOr); node->data.suspend.block = ast_parse_block(pc, token_index, true); -- cgit v1.2.3