From e5fcdfadc5240d9d8a23dec51d4397b040537c01 Mon Sep 17 00:00:00 2001 From: g-w1 Date: Wed, 23 Dec 2020 15:47:41 -0500 Subject: [PATCH] few small things based on review: * make Type.eql for errors not reach unreachable * use global_error_set instead of getErrorValue * use value in analyzeInstCmp instead of type and use the u16 error value field instead of the name so that we don't need mem.eql * clean up code by factoring code from Module.zig -> type.zig so that it can be reused in peer type resolution and coercion * update after rebasing and fix dumb things that I saw when reviewing my own code --- src/Module.zig | 63 +++----------------------------------------- src/type.zig | 56 +++++++++++++++++++++++++++++++++++---- src/value.zig | 2 +- src/zir_sema.zig | 38 +++++++++++++------------- test/stage2/test.zig | 4 +-- 5 files changed, 77 insertions(+), 86 deletions(-) diff --git a/src/Module.zig b/src/Module.zig index 94a578800bb7..ca5d6dbe7acc 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -2753,6 +2753,7 @@ pub fn resolvePeerTypes(self: *Module, scope: *Scope, instructions: []*Inst) !Ty chosen = candidate; continue; } + if (chosen.ty.isInt() and candidate.ty.isInt() and chosen.ty.isSignedInt() == candidate.ty.isSignedInt()) @@ -2865,65 +2866,9 @@ pub fn coerce(self: *Module, scope: *Scope, dest_type: Type, inst: *Inst) !*Inst // error set widening if (inst.ty.zigTypeTag() == .ErrorSet and dest_type.zigTypeTag() == .ErrorSet) { - const gotten_err_set = inst.ty.getErrs(); - switch (dest_type.tag()) { - .error_set => { - switch (gotten_err_set) { - .multiple => |fields| { - const dt_e = dest_type.getErrs().multiple; - // we can do > because if they were equal then Type.eql wouldn't have let control flow get this far - if (dt_e.size > fields.size) blk: { - var it = fields.iterator(); - while (it.next()) |entry| { - if (dt_e.get(entry.key) == null) break :blk; // the smaller set has a key not in the larger set - } - var it_dest = dt_e.iterator(); - while (it_dest.next()) |entry| { - try fields.put(self.gpa, entry.key, entry.value); - } - } - return inst; - }, - .err_single => |name| blk: { // dont have to deinit anything because it is a slice - const dt_e = dest_type.getErrs().multiple; - if (dt_e.get(name) == null) break :blk; // the smaller set has a key not in the larger set - var new_decl_arena = std.heap.ArenaAllocator.init(self.gpa); - errdefer new_decl_arena.deinit(); - const payload_val = try scope.arena().create(Value.Payload.ErrorSet); - payload_val.* = .{ - .fields = .{}, - .decl = undefined, // populated below - }; - payload_val.fields = try dt_e.clone(&new_decl_arena.allocator); - // TODO create name in format "error:line:column" - const new_decl = try self.createAnonymousDecl(scope, &new_decl_arena, .{ - .ty = Type.initTag(.type), - .val = Value.initPayload(&payload_val.base), - }); - payload_val.decl = new_decl; - const payload_ty = try scope.arena().create(Type.Payload.ErrorSet); - payload_ty.* = .{ .decl = new_decl }; - inst.ty = Type.initPayload(&payload_ty.base); - return inst; - }, - else => unreachable, - } - }, - .error_set_single => {}, // we do nothing, because if they were equal Type.eql would have caught and if not, then not coercible - .anyerror => { - switch (gotten_err_set) { - .multiple => |fields| { - inst.ty = Type.initTag(.anyerror); - return inst; - }, - .err_single => |_| { - inst.ty = Type.initTag(.anyerror); - return inst; - }, - else => unreachable, - } - }, - else => unreachable, + if (dest_type.errorSetFitsInAnother(inst.ty)) { + inst.ty = dest_type; + return inst; } } diff --git a/src/type.zig b/src/type.zig index 73f60bebb2a0..7e6294a75a8e 100644 --- a/src/type.zig +++ b/src/type.zig @@ -267,6 +267,8 @@ pub const Type = extern union { return true else return false; + } else if (b.tag() == .anyerror) { + return false; } if (a.tag() == .error_set_single and b.tag() == .error_set_single) { @@ -301,9 +303,9 @@ pub const Type = extern union { unreachable; }, .ErrorUnion => { - const a_casted = a.cast(Payload.ErrorUnion).?; - const b_casted = b.cast(Payload.ErrorUnion).?; - return a_casted.error_set.eql(b_casted.error_set) and a_casted.payload.eql(b_casted.payload); + const a_data = a.castTag(.error_union).?.data; + const b_data = b.castTag(.error_union).?.data; + return a_data.error_set.eql(b_data.error_set) and a_data.payload.eql(b_data.payload); }, .Float, .Struct, @@ -1705,12 +1707,56 @@ pub const Type = extern union { .const_slice_u8, .single_const_pointer_to_comptime_int, .pointer, + .inferred_alloc_mut, + .inferred_alloc_const, => unreachable, .anyerror => return .{ .anyerror = {} }, - .error_set_single => return .{ .err_single = self.cast(Payload.ErrorSetSingle).?.name }, - .error_set => return .{ .multiple = &self.cast(Payload.ErrorSet).?.decl.typed_value.most_recent.typed_value.val.cast(Value.Payload.ErrorSet).?.fields }, + .error_set_single => return .{ .err_single = self.castTag(.error_set_single).?.data }, + .error_set => return .{ .multiple = &self.castTag(.error_set).?.data.typed_value.most_recent.typed_value.val.castTag(.error_set).?.data.fields }, }; } + /// Asserts the type is error_set or error_set_single or anyerror and that if it is error_set, it's decl has been analyzed. + /// Returns true if fitee can fit into fitter, and false if not. + pub fn errorSetFitsInAnother(fitter: Type, fitee: Type) bool { + if (fitter.eql(fitee)) return true; + switch (fitee.getErrs()) { + .multiple => |fitee_set| { + switch (fitter.getErrs()) { + .multiple => |fitter_set| { + // we can do < because if they were equal then Type.eql wouldn't have let control flow get this far + if (fitee_set.size < fitter_set.size) { + var it = fitter_set.iterator(); + while (it.next()) |entry| { + // the smaller set has a key not in the larger set + if (fitee_set.get(entry.key) == null) return false; + } + return true; + } else return false; + }, + // we return false, because if they were equal Type.eql would have caught and if not, then not coercible + .err_single => return false, + // any set can fit into anyerror + .anyerror => return true, + } + }, + .err_single => |fitee_name| { + switch (fitter.getErrs()) { + .multiple => |fitter_set| { + // the smaller set has a key not in the larger set + if (fitter_set.get(fitee_name) == null) return false; + return true; + }, + // we return false, because if they were equal Type.eql would have caught and if not, then not coercible + .err_single => return false, + // any set can fit into anyerror + .anyerror => return true, + } + }, + + // if they were both anyerror, then Type.eql would have caught it, and if not, then nothing else can fit into anyerorr + .anyerror => return false, + } + } /// Asserts the type is a pointer or array type. pub fn elemType(self: Type) Type { return switch (self.tag()) { diff --git a/src/value.zig b/src/value.zig index 78d568ecf086..c6eac57ce272 100644 --- a/src/value.zig +++ b/src/value.zig @@ -1381,7 +1381,7 @@ pub const Value = extern union { const b_name = b.castTag(.enum_literal).?.data; return std.mem.eql(u8, a_name, b_name); } else if (a.tag() == .@"error" and b.tag() == .@"error") { - return a.cast(Payload.Error).?.value == b.cast(Payload.Error).?.value; + return a.castTag(.@"error").?.data.value == b.castTag(.@"error").?.data.value; } } if (a.isType() and b.isType()) { diff --git a/src/zir_sema.zig b/src/zir_sema.zig index 95f320a5c9ff..365b07247a50 100644 --- a/src/zir_sema.zig +++ b/src/zir_sema.zig @@ -944,14 +944,17 @@ fn analyzeInstMergeErrorSets(mod: *Module, scope: *Scope, inst: *zir.Inst.BinOp) const payload = try scope.arena().create(Value.Payload.ErrorSet); payload.* = .{ - .fields = .{}, - .decl = undefined, // populated below + .base = .{ .tag = .error_set }, + .data = .{ + .fields = .{}, + .decl = undefined, // populated below + }, }; - try payload.fields.ensureCapacity(&new_decl_arena.allocator, @intCast(u32, switch (rhs_fields) { + try payload.data.fields.ensureCapacity(&new_decl_arena.allocator, @intCast(u32, switch (rhs_fields) { .err_single => 1, .multiple => |mul| mul.size, else => unreachable, - } + switch (rhs_fields) { + } + switch (lhs_fields) { .err_single => 1, .multiple => |mul| mul.size, else => unreachable, @@ -959,13 +962,13 @@ fn analyzeInstMergeErrorSets(mod: *Module, scope: *Scope, inst: *zir.Inst.BinOp) switch (lhs_fields) { .err_single => |name| { - const entry = mod.global_error_set.get(name).?; - payload.fields.putAssumeCapacity(name, entry); + const num = mod.global_error_set.get(name).?; + payload.data.fields.putAssumeCapacity(name, num); }, .multiple => |multiple| { var it = multiple.iterator(); while (it.next()) |entry| { - payload.fields.putAssumeCapacity(entry.key, entry.value); + payload.data.fields.putAssumeCapacity(entry.key, entry.value); } }, else => unreachable, @@ -973,14 +976,14 @@ fn analyzeInstMergeErrorSets(mod: *Module, scope: *Scope, inst: *zir.Inst.BinOp) switch (rhs_fields) { .err_single => |name| { - const entry = try mod.getErrorValue(name); - payload.fields.putAssumeCapacity(entry.key, entry.value); + const num = mod.global_error_set.get(name).?; + payload.data.fields.putAssumeCapacity(name, num); }, .multiple => |multiple| { var it = multiple.iterator(); while (it.next()) |name| { const entry = try mod.getErrorValue(name.key); - payload.fields.putAssumeCapacity(entry.key, entry.value); + payload.data.fields.putAssumeCapacity(entry.key, entry.value); } }, else => unreachable, @@ -989,7 +992,7 @@ fn analyzeInstMergeErrorSets(mod: *Module, scope: *Scope, inst: *zir.Inst.BinOp) .ty = Type.initTag(.type), .val = Value.initPayload(&payload.base), }); - payload.decl = new_decl; + payload.data.decl = new_decl; return mod.constInst(scope, inst.base.src, .{ .ty = Type.initTag(.type), @@ -1463,7 +1466,7 @@ fn validateSwitch(mod: *Module, scope: *Scope, target: *Inst, inst: *zir.Inst.Sw const resolved = try resolveInst(mod, scope, item); const casted = try mod.coerce(scope, target.ty, resolved); const val = try mod.resolveConstValue(scope, casted); - const err_name = val.cast(Value.Payload.Error).?.name; + const err_name = val.castTag(.@"error").?.data.name; if (try seen_values.fetchPut(val, item.src)) |prev| { return mod.fail(scope, item.src, "duplicate switch value", .{}); } @@ -1876,13 +1879,10 @@ fn analyzeInstCmp( if (!is_equality_cmp) { return mod.fail(scope, inst.base.src, "{} operator not allowed for errors", .{@tagName(op)}); } - const lhs_comptime = lhs.ty.cast(Type.Payload.ErrorSetSingle); - const rhs_comptime = rhs.ty.cast(Type.Payload.ErrorSetSingle); - if (lhs_comptime != null and rhs_comptime != null) { - if (op == .eq) - return mod.constBool(scope, inst.base.src, !std.mem.eql(u8, lhs_comptime.?.name, rhs_comptime.?.name)) - else - return mod.constBool(scope, inst.base.src, std.mem.eql(u8, lhs_comptime.?.name, rhs_comptime.?.name)); + const lhs_val = lhs.value(); + const rhs_val = rhs.value(); + if (lhs_val != null and rhs_val != null) { + return mod.constBool(scope, inst.base.src, (lhs_val.?.castTag(.@"error").?.data.value == rhs_val.?.castTag(.@"error").?.data.value) == (op == .eq)); } return mod.fail(scope, inst.base.src, "TODO runtime error comparison", .{}); } else if (lhs.ty.isNumeric() and rhs.ty.isNumeric()) { diff --git a/test/stage2/test.zig b/test/stage2/test.zig index 734606ca0bf6..a8a8edd59873 100644 --- a/test/stage2/test.zig +++ b/test/stage2/test.zig @@ -362,7 +362,7 @@ pub fn addCases(ctx: *TestContext) !void { \\ } \\ unreachable; // because it will give error above \\} - , &[_][]const u8{":6:14: error: expected _start__anon_13, found error{Z}"}); + , &[_][]const u8{":6:14: error: expected _start__anon_12, found error{Z}"}); case.addError( \\export fn _start() noreturn { \\ const T = error{ A, B, C, D }; @@ -501,7 +501,7 @@ pub fn addCases(ctx: *TestContext) !void { var case = ctx.exe("comptime error equality", linux_x64); case.addCompareOutput( \\export fn _start() noreturn { - \\ if (error.V == error.T) { + \\ if (error.T == error.T) { \\ condPrint(); \\ } \\