From d84282174c449325b4bc43fed4d65289445399d4 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 17 Aug 2022 09:00:23 +0200 Subject: [PATCH 01/10] link-test: move tls test to macho/tls This test was always really testing correct behavior of our in-house MachO linker to begin with. --- test/link.zig | 13 ++++--------- test/link/{ => macho}/tls/a.c | 0 test/link/{ => macho}/tls/build.zig | 1 + test/link/{ => macho}/tls/main.zig | 0 4 files changed, 5 insertions(+), 9 deletions(-) rename test/link/{ => macho}/tls/a.c (100%) rename test/link/{ => macho}/tls/build.zig (88%) rename test/link/{ => macho}/tls/main.zig (100%) diff --git a/test/link.zig b/test/link.zig index 01edd07261..a8a39a7018 100644 --- a/test/link.zig +++ b/test/link.zig @@ -3,11 +3,6 @@ const builtin = @import("builtin"); const tests = @import("tests.zig"); pub fn addCases(cases: *tests.StandaloneContext) void { - if (builtin.os.tag == .windows) { - // https://github.com/ziglang/zig/issues/12421 - return; - } - cases.addBuildFile("test/link/bss/build.zig", .{ .build_modes = false, // we only guarantee zerofill for undefined in Debug }); @@ -28,10 +23,6 @@ pub fn addCases(cases: *tests.StandaloneContext) void { .build_modes = true, }); - cases.addBuildFile("test/link/tls/build.zig", .{ - .build_modes = true, - }); - cases.addBuildFile("test/link/wasm/type/build.zig", .{ .build_modes = true, .requires_stage2 = true, @@ -115,4 +106,8 @@ pub fn addCases(cases: *tests.StandaloneContext) void { .build_modes = true, .requires_macos_sdk = true, }); + + cases.addBuildFile("test/link/macho/tls/build.zig", .{ + .build_modes = true, + }); } diff --git a/test/link/tls/a.c b/test/link/macho/tls/a.c similarity index 100% rename from test/link/tls/a.c rename to test/link/macho/tls/a.c diff --git a/test/link/tls/build.zig b/test/link/macho/tls/build.zig similarity index 88% rename from test/link/tls/build.zig rename to test/link/macho/tls/build.zig index ebf15ca439..7bf3ea1c9e 100644 --- a/test/link/tls/build.zig +++ b/test/link/macho/tls/build.zig @@ -2,6 +2,7 @@ const Builder = @import("std").build.Builder; pub fn build(b: *Builder) void { const mode = b.standardReleaseOptions(); + const target: std.zig.CrossTarget = .{ .os_tag = .macos }; const lib = b.addSharedLibrary("a", null, b.version(1, 0, 0)); lib.setBuildMode(mode); diff --git a/test/link/tls/main.zig b/test/link/macho/tls/main.zig similarity index 100% rename from test/link/tls/main.zig rename to test/link/macho/tls/main.zig From c764640e92c9e4d32b89650ac774bebf1498be92 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 17 Aug 2022 12:55:08 -0700 Subject: [PATCH 02/10] Sema: fix generics with struct literal coerced to tagged union The `Value.eql` function has to test for value equality *as-if* the lhs value parameter is coerced into the type of the rhs. For tagged unions, there was a problematic case when the lhs was an anonymous struct, because in such case the value is empty_struct_value and the type contains all the value information. But the only type available in the function was the rhs type. So the fix involved making `Value.eqlAdvanced` also accept the lhs type, and then enhancing the logic to handle the case of the `.anon_struct` tag. closes #12418 Tests run locally: * test-behavior * test-cases --- src/Sema.zig | 56 +++++++++++++++++---------- src/value.zig | 77 ++++++++++++++++++++++++++------------ test/behavior/generics.zig | 19 ++++++++++ 3 files changed, 110 insertions(+), 42 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index d7d6994bcd..f1d140520c 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -1495,7 +1495,8 @@ pub fn resolveInst(sema: *Sema, zir_ref: Zir.Inst.Ref) !Air.Inst.Ref { // Finally, the last section of indexes refers to the map of ZIR=>AIR. const inst = sema.inst_map.get(@intCast(u32, i)).?; - if (sema.typeOf(inst).tag() == .generic_poison) return error.GenericPoison; + const ty = sema.typeOf(inst); + if (ty.tag() == .generic_poison) return error.GenericPoison; return inst; } @@ -5570,11 +5571,15 @@ const GenericCallAdapter = struct { generic_fn: *Module.Fn, precomputed_hash: u64, func_ty_info: Type.Payload.Function.Data, - /// Unlike comptime_args, the Type here is not always present. - /// .generic_poison is used to communicate non-anytype parameters. - comptime_tvs: []const TypedValue, + args: []const Arg, module: *Module, + const Arg = struct { + ty: Type, + val: Value, + is_anytype: bool, + }; + pub fn eql(ctx: @This(), adapted_key: void, other_key: *Module.Fn) bool { _ = adapted_key; // The generic function Decl is guaranteed to be the first dependency @@ -5585,10 +5590,10 @@ const GenericCallAdapter = struct { const other_comptime_args = other_key.comptime_args.?; for (other_comptime_args[0..ctx.func_ty_info.param_types.len]) |other_arg, i| { - const this_arg = ctx.comptime_tvs[i]; + const this_arg = ctx.args[i]; const this_is_comptime = this_arg.val.tag() != .generic_poison; const other_is_comptime = other_arg.val.tag() != .generic_poison; - const this_is_anytype = this_arg.ty.tag() != .generic_poison; + const this_is_anytype = this_arg.is_anytype; const other_is_anytype = other_key.isAnytypeParam(ctx.module, @intCast(u32, i)); if (other_is_anytype != this_is_anytype) return false; @@ -5607,7 +5612,17 @@ const GenericCallAdapter = struct { } } else if (this_is_comptime) { // Both are comptime parameters but not anytype parameters. - if (!this_arg.val.eql(other_arg.val, other_arg.ty, ctx.module)) { + // We assert no error is possible here because any lazy values must be resolved + // before inserting into the generic function hash map. + const is_eql = Value.eqlAdvanced( + this_arg.val, + this_arg.ty, + other_arg.val, + other_arg.ty, + ctx.module, + null, + ) catch unreachable; + if (!is_eql) { return false; } } @@ -6258,8 +6273,7 @@ fn instantiateGenericCall( var hasher = std.hash.Wyhash.init(0); std.hash.autoHash(&hasher, @ptrToInt(module_fn)); - const comptime_tvs = try sema.arena.alloc(TypedValue, func_ty_info.param_types.len); - + const generic_args = try sema.arena.alloc(GenericCallAdapter.Arg, func_ty_info.param_types.len); { var i: usize = 0; for (fn_info.param_body) |inst| { @@ -6283,8 +6297,9 @@ fn instantiateGenericCall( else => continue, } + const arg_ty = sema.typeOf(uncasted_args[i]); + if (is_comptime) { - const arg_ty = sema.typeOf(uncasted_args[i]); const arg_val = sema.analyzeGenericCallArgVal(block, .unneeded, uncasted_args[i]) catch |err| switch (err) { error.NeededSourceLocation => { const decl = sema.mod.declPtr(block.src_decl); @@ -6297,27 +6312,30 @@ fn instantiateGenericCall( arg_val.hash(arg_ty, &hasher, mod); if (is_anytype) { arg_ty.hashWithHasher(&hasher, mod); - comptime_tvs[i] = .{ + generic_args[i] = .{ .ty = arg_ty, .val = arg_val, + .is_anytype = true, }; } else { - comptime_tvs[i] = .{ - .ty = Type.initTag(.generic_poison), + generic_args[i] = .{ + .ty = arg_ty, .val = arg_val, + .is_anytype = false, }; } } else if (is_anytype) { - const arg_ty = sema.typeOf(uncasted_args[i]); arg_ty.hashWithHasher(&hasher, mod); - comptime_tvs[i] = .{ + generic_args[i] = .{ .ty = arg_ty, .val = Value.initTag(.generic_poison), + .is_anytype = true, }; } else { - comptime_tvs[i] = .{ - .ty = Type.initTag(.generic_poison), + generic_args[i] = .{ + .ty = arg_ty, .val = Value.initTag(.generic_poison), + .is_anytype = false, }; } @@ -6331,7 +6349,7 @@ fn instantiateGenericCall( .generic_fn = module_fn, .precomputed_hash = precomputed_hash, .func_ty_info = func_ty_info, - .comptime_tvs = comptime_tvs, + .args = generic_args, .module = mod, }; const gop = try mod.monomorphed_funcs.getOrPutAdapted(gpa, {}, adapter); @@ -30124,7 +30142,7 @@ fn valuesEqual( rhs: Value, ty: Type, ) CompileError!bool { - return Value.eqlAdvanced(lhs, rhs, ty, sema.mod, sema.kit(block, src)); + return Value.eqlAdvanced(lhs, ty, rhs, ty, sema.mod, sema.kit(block, src)); } /// Asserts the values are comparable vectors of type `ty`. diff --git a/src/value.zig b/src/value.zig index 677a459afe..9909cab5ce 100644 --- a/src/value.zig +++ b/src/value.zig @@ -2004,6 +2004,10 @@ pub const Value = extern union { return (try orderAgainstZeroAdvanced(lhs, sema_kit)).compare(op); } + pub fn eql(a: Value, b: Value, ty: Type, mod: *Module) bool { + return eqlAdvanced(a, ty, b, ty, mod, null) catch unreachable; + } + /// This function is used by hash maps and so treats floating-point NaNs as equal /// to each other, and not equal to other floating-point values. /// Similarly, it treats `undef` as a distinct value from all other values. @@ -2012,13 +2016,10 @@ pub const Value = extern union { /// for `a`. This function must act *as if* `a` has been coerced to `ty`. This complication /// is required in order to make generic function instantiation efficient - specifically /// the insertion into the monomorphized function table. - pub fn eql(a: Value, b: Value, ty: Type, mod: *Module) bool { - return eqlAdvanced(a, b, ty, mod, null) catch unreachable; - } - /// If `null` is provided for `sema_kit` then it is guaranteed no error will be returned. pub fn eqlAdvanced( a: Value, + a_ty: Type, b: Value, ty: Type, mod: *Module, @@ -2044,33 +2045,34 @@ pub const Value = extern union { const a_payload = a.castTag(.opt_payload).?.data; const b_payload = b.castTag(.opt_payload).?.data; var buffer: Type.Payload.ElemType = undefined; - return eqlAdvanced(a_payload, b_payload, ty.optionalChild(&buffer), mod, sema_kit); + const payload_ty = ty.optionalChild(&buffer); + return eqlAdvanced(a_payload, payload_ty, b_payload, payload_ty, mod, sema_kit); }, .slice => { const a_payload = a.castTag(.slice).?.data; const b_payload = b.castTag(.slice).?.data; - if (!(try eqlAdvanced(a_payload.len, b_payload.len, Type.usize, mod, sema_kit))) { + if (!(try eqlAdvanced(a_payload.len, Type.usize, b_payload.len, Type.usize, mod, sema_kit))) { return false; } var ptr_buf: Type.SlicePtrFieldTypeBuffer = undefined; const ptr_ty = ty.slicePtrFieldType(&ptr_buf); - return eqlAdvanced(a_payload.ptr, b_payload.ptr, ptr_ty, mod, sema_kit); + return eqlAdvanced(a_payload.ptr, ptr_ty, b_payload.ptr, ptr_ty, mod, sema_kit); }, .elem_ptr => { const a_payload = a.castTag(.elem_ptr).?.data; const b_payload = b.castTag(.elem_ptr).?.data; if (a_payload.index != b_payload.index) return false; - return eqlAdvanced(a_payload.array_ptr, b_payload.array_ptr, ty, mod, sema_kit); + return eqlAdvanced(a_payload.array_ptr, ty, b_payload.array_ptr, ty, mod, sema_kit); }, .field_ptr => { const a_payload = a.castTag(.field_ptr).?.data; const b_payload = b.castTag(.field_ptr).?.data; if (a_payload.field_index != b_payload.field_index) return false; - return eqlAdvanced(a_payload.container_ptr, b_payload.container_ptr, ty, mod, sema_kit); + return eqlAdvanced(a_payload.container_ptr, ty, b_payload.container_ptr, ty, mod, sema_kit); }, .@"error" => { const a_name = a.castTag(.@"error").?.data.name; @@ -2080,7 +2082,8 @@ pub const Value = extern union { .eu_payload => { const a_payload = a.castTag(.eu_payload).?.data; const b_payload = b.castTag(.eu_payload).?.data; - return eqlAdvanced(a_payload, b_payload, ty.errorUnionPayload(), mod, sema_kit); + const payload_ty = ty.errorUnionPayload(); + return eqlAdvanced(a_payload, payload_ty, b_payload, payload_ty, mod, sema_kit); }, .eu_payload_ptr => @panic("TODO: Implement more pointer eql cases"), .opt_payload_ptr => @panic("TODO: Implement more pointer eql cases"), @@ -2098,7 +2101,7 @@ pub const Value = extern union { const types = ty.tupleFields().types; assert(types.len == a_field_vals.len); for (types) |field_ty, i| { - if (!(try eqlAdvanced(a_field_vals[i], b_field_vals[i], field_ty, mod, sema_kit))) { + if (!(try eqlAdvanced(a_field_vals[i], field_ty, b_field_vals[i], field_ty, mod, sema_kit))) { return false; } } @@ -2109,7 +2112,7 @@ pub const Value = extern union { const fields = ty.structFields().values(); assert(fields.len == a_field_vals.len); for (fields) |field, i| { - if (!(try eqlAdvanced(a_field_vals[i], b_field_vals[i], field.ty, mod, sema_kit))) { + if (!(try eqlAdvanced(a_field_vals[i], field.ty, b_field_vals[i], field.ty, mod, sema_kit))) { return false; } } @@ -2120,7 +2123,7 @@ pub const Value = extern union { for (a_field_vals) |a_elem, i| { const b_elem = b_field_vals[i]; - if (!(try eqlAdvanced(a_elem, b_elem, elem_ty, mod, sema_kit))) { + if (!(try eqlAdvanced(a_elem, elem_ty, b_elem, elem_ty, mod, sema_kit))) { return false; } } @@ -2132,7 +2135,7 @@ pub const Value = extern union { switch (ty.containerLayout()) { .Packed, .Extern => { const tag_ty = ty.unionTagTypeHypothetical(); - if (!(try a_union.tag.eqlAdvanced(b_union.tag, tag_ty, mod, sema_kit))) { + if (!(try eqlAdvanced(a_union.tag, tag_ty, b_union.tag, tag_ty, mod, sema_kit))) { // In this case, we must disregard mismatching tags and compare // based on the in-memory bytes of the payloads. @panic("TODO comptime comparison of extern union values with mismatching tags"); @@ -2140,13 +2143,13 @@ pub const Value = extern union { }, .Auto => { const tag_ty = ty.unionTagTypeHypothetical(); - if (!(try a_union.tag.eqlAdvanced(b_union.tag, tag_ty, mod, sema_kit))) { + if (!(try eqlAdvanced(a_union.tag, tag_ty, b_union.tag, tag_ty, mod, sema_kit))) { return false; } }, } const active_field_ty = ty.unionFieldType(a_union.tag, mod); - return a_union.val.eqlAdvanced(b_union.val, active_field_ty, mod, sema_kit); + return eqlAdvanced(a_union.val, active_field_ty, b_union.val, active_field_ty, mod, sema_kit); }, else => {}, } else if (a_tag == .null_value or b_tag == .null_value) { @@ -2180,7 +2183,7 @@ pub const Value = extern union { const b_val = b.enumToInt(ty, &buf_b); var buf_ty: Type.Payload.Bits = undefined; const int_ty = ty.intTagType(&buf_ty); - return eqlAdvanced(a_val, b_val, int_ty, mod, sema_kit); + return eqlAdvanced(a_val, int_ty, b_val, int_ty, mod, sema_kit); }, .Array, .Vector => { const len = ty.arrayLen(); @@ -2191,17 +2194,44 @@ pub const Value = extern union { while (i < len) : (i += 1) { const a_elem = elemValueBuffer(a, mod, i, &a_buf); const b_elem = elemValueBuffer(b, mod, i, &b_buf); - if (!(try eqlAdvanced(a_elem, b_elem, elem_ty, mod, sema_kit))) { + if (!(try eqlAdvanced(a_elem, elem_ty, b_elem, elem_ty, mod, sema_kit))) { return false; } } return true; }, .Struct => { - // A tuple can be represented with .empty_struct_value, - // the_one_possible_value, .aggregate in which case we could - // end up here and the values are equal if the type has zero fields. - return ty.isTupleOrAnonStruct() and ty.structFieldCount() != 0; + // A struct can be represented with one of: + // .empty_struct_value, + // .the_one_possible_value, + // .aggregate, + // Note that we already checked above for matching tags, e.g. both .aggregate. + return ty.onePossibleValue() != null; + }, + .Union => { + // Here we have to check for value equality, as-if `a` has been coerced to `ty`. + if (ty.onePossibleValue() != null) { + return true; + } + if (a_ty.castTag(.anon_struct)) |payload| { + const tuple = payload.data; + if (tuple.values.len != 1) { + return false; + } + const field_name = tuple.names[0]; + const union_obj = ty.cast(Type.Payload.Union).?.data; + const field_index = union_obj.fields.getIndex(field_name) orelse return false; + const tag_and_val = b.castTag(.@"union").?.data; + var field_tag_buf: Value.Payload.U32 = .{ + .base = .{ .tag = .enum_field_index }, + .data = @intCast(u32, field_index), + }; + const field_tag = Value.initPayload(&field_tag_buf.base); + const tag_matches = tag_and_val.tag.eql(field_tag, union_obj.tag_ty, mod); + if (!tag_matches) return false; + return eqlAdvanced(tag_and_val.val, union_obj.tag_ty, tuple.values[0], tuple.types[0], mod, sema_kit); + } + return false; }, .Float => { switch (ty.floatBits(target)) { @@ -2230,7 +2260,8 @@ pub const Value = extern union { .base = .{ .tag = .opt_payload }, .data = a, }; - return eqlAdvanced(Value.initPayload(&buffer.base), b, ty, mod, sema_kit); + const opt_val = Value.initPayload(&buffer.base); + return eqlAdvanced(opt_val, ty, b, ty, mod, sema_kit); } }, else => {}, diff --git a/test/behavior/generics.zig b/test/behavior/generics.zig index cd4e018be3..d930fb7d27 100644 --- a/test/behavior/generics.zig +++ b/test/behavior/generics.zig @@ -323,3 +323,22 @@ test "generic function instantiation non-duplicates" { S.copy(u8, &buffer, "hello"); S.copy(u8, &buffer, "hello2"); } + +test "generic instantiation of tagged union with only one field" { + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.os.tag == .wasi) return error.SkipZigTest; + + const S = struct { + const U = union(enum) { + s: []const u8, + }; + + fn foo(comptime u: U) usize { + return u.s.len; + } + }; + + try expect(S.foo(.{ .s = "a" }) == 1); + try expect(S.foo(.{ .s = "ab" }) == 2); +} From 070282a96ec23fb41041843d5753608ec5090f8b Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 17 Aug 2022 16:58:16 +0200 Subject: [PATCH 03/10] libstd: fix off-by-one error in def of ProcSym in pdb Make sure `ProcSym` includes a single element byte-array which delimits the start of the symbol's name as part of its definition. This makes the code more elegant in that accessing the name is equivalent to taking the address of this one element array. --- lib/std/pdb.zig | 9 ++++++--- test/stack_traces.zig | 7 +------ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/std/pdb.zig b/lib/std/pdb.zig index a44296c920..00ce2cc5ba 100644 --- a/lib/std/pdb.zig +++ b/lib/std/pdb.zig @@ -310,6 +310,10 @@ pub const SymbolKind = enum(u16) { pub const TypeIndex = u32; +// TODO According to this header: +// https://github.com/microsoft/microsoft-pdb/blob/082c5290e5aff028ae84e43affa8be717aa7af73/include/cvinfo.h#L3722 +// we should define RecordPrefix as part of the ProcSym structure. +// This might be important when we start generating PDB in self-hosted with our own PE linker. pub const ProcSym = extern struct { Parent: u32, End: u32, @@ -321,8 +325,7 @@ pub const ProcSym = extern struct { CodeOffset: u32, Segment: u16, Flags: ProcSymFlags, - // following is a null terminated string - // Name: [*]u8, + Name: [1]u8, // null-terminated }; pub const ProcSymFlags = packed struct { @@ -693,7 +696,7 @@ pub const Pdb = struct { .S_LPROC32, .S_GPROC32 => { const proc_sym = @ptrCast(*align(1) ProcSym, &module.symbols[symbol_i + @sizeOf(RecordPrefix)]); if (address >= proc_sym.CodeOffset and address < proc_sym.CodeOffset + proc_sym.CodeSize) { - return mem.sliceTo(@ptrCast([*:0]u8, proc_sym) + @sizeOf(ProcSym), 0); + return mem.sliceTo(@ptrCast([*:0]u8, &proc_sym.Name[0]), 0); } }, else => {}, diff --git a/test/stack_traces.zig b/test/stack_traces.zig index e514e0fd88..06534a2f7e 100644 --- a/test/stack_traces.zig +++ b/test/stack_traces.zig @@ -3,11 +3,6 @@ const os = std.os; const tests = @import("tests.zig"); pub fn addCases(cases: *tests.StackTracesContext) void { - if (@import("builtin").os.tag == .windows) { - // https://github.com/ziglang/zig/issues/12422 - return; - } - cases.addCase(.{ .name = "return", .source = @@ -178,7 +173,7 @@ pub fn addCases(cases: *tests.StackTracesContext) void { cases.addCase(.{ .exclude_os = .{ .openbsd, // integer overflow - .windows, + .windows, // TODO intermittent failures }, .name = "dumpCurrentStackTrace", .source = From 07f64a2e13ab80acffba7f7bdd5d7c58df7893c0 Mon Sep 17 00:00:00 2001 From: Martin Hafskjold Thoresen Date: Wed, 10 Aug 2022 23:32:02 +0200 Subject: [PATCH 04/10] Sema: error on ambiguous coercion of comptime float and ints The following, from the documentation as of the time of writing, illustrates the problem: ```zig // Compile time coercion of float to int test "implicit cast to comptime_int" { var f: f32 = 54.0 / 5; _ = f; } ``` It is not clear how to unify the types of 54.0 and 5 to perform the division. We can either - cast 54.0 to comptime_int resulting in @as(comptime_int, 10), which is casted to @as(f32, 10), or - cast 5 to comptime_float resulting in @as(comptime_float, 10.8), which is casted to @as(f32, 10.8) Since the two resulting values are different, a compiler error is appropriate. If we know that casting to either type will result in the same value we don't need to error. For instance, 10.0 / 2 is okay, as is 10 / 2.0. Fixes: #12364 --- src/Sema.zig | 16 ++++++++++++++++ test/behavior/floatop.zig | 8 ++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index f1d140520c..e5700a6fe4 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -11202,6 +11202,22 @@ fn zirDiv(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins const maybe_lhs_val = try sema.resolveMaybeUndefValIntable(block, lhs_src, casted_lhs); const maybe_rhs_val = try sema.resolveMaybeUndefValIntable(block, rhs_src, casted_rhs); + if ((lhs_ty.tag() == .comptime_float and rhs_ty.tag() == .comptime_int) or + (lhs_ty.tag() == .comptime_int and rhs_ty.tag() == .comptime_float)) + { + // If it makes a difference whether we coerce to ints or floats before doing the division, error. + // If lhs % rhs is 0, it doesn't matter. + var lhs_val = maybe_lhs_val orelse unreachable; + var rhs_val = maybe_rhs_val orelse unreachable; + var rem = lhs_val.floatRem(rhs_val, resolved_type, sema.arena, target) catch unreachable; + var float_rem = rem.toFloat(f32); + if (float_rem != 0.0) { + return sema.fail(block, src, "ambiguous coercion of division operands: '{s}' and '{s}': division has non-zero reminder: {d}", .{ + @tagName(lhs_ty.tag()), @tagName(rhs_ty.tag()), float_rem, + }); + } + } + // TODO: emit compile error when .div is used on integers and there would be an // ambiguous result between div_floor and div_trunc. diff --git a/test/behavior/floatop.zig b/test/behavior/floatop.zig index c057f7a842..a5eb25d4f5 100644 --- a/test/behavior/floatop.zig +++ b/test/behavior/floatop.zig @@ -194,8 +194,8 @@ fn testSin() !void { const eps = epsForType(ty); try expect(@sin(@as(ty, 0)) == 0); try expect(math.approxEqAbs(ty, @sin(@as(ty, std.math.pi)), 0, eps)); - try expect(math.approxEqAbs(ty, @sin(@as(ty, std.math.pi / 2)), 1, eps)); - try expect(math.approxEqAbs(ty, @sin(@as(ty, std.math.pi / 4)), 0.7071067811865475, eps)); + try expect(math.approxEqAbs(ty, @sin(@as(ty, std.math.pi / 2.0)), 1, eps)); + try expect(math.approxEqAbs(ty, @sin(@as(ty, std.math.pi / 4.0)), 0.7071067811865475, eps)); } { @@ -228,8 +228,8 @@ fn testCos() !void { const eps = epsForType(ty); try expect(@cos(@as(ty, 0)) == 1); try expect(math.approxEqAbs(ty, @cos(@as(ty, std.math.pi)), -1, eps)); - try expect(math.approxEqAbs(ty, @cos(@as(ty, std.math.pi / 2)), 0, eps)); - try expect(math.approxEqAbs(ty, @cos(@as(ty, std.math.pi / 4)), 0.7071067811865475, eps)); + try expect(math.approxEqAbs(ty, @cos(@as(ty, std.math.pi / 2.0)), 0, eps)); + try expect(math.approxEqAbs(ty, @cos(@as(ty, std.math.pi / 4.0)), 0.7071067811865475, eps)); } { From 59b6483d63ca68f63c0b3758e80da918629ada2c Mon Sep 17 00:00:00 2001 From: Veikka Tuominen Date: Tue, 16 Aug 2022 16:17:56 +0300 Subject: [PATCH 05/10] add test --- src/Sema.zig | 17 +++++++------ ...mbiguous_coercion_of_division_operands.zig | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+), 9 deletions(-) create mode 100644 test/cases/compile_errors/ambiguous_coercion_of_division_operands.zig diff --git a/src/Sema.zig b/src/Sema.zig index e5700a6fe4..29a53ebb2a 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -11202,18 +11202,17 @@ fn zirDiv(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins const maybe_lhs_val = try sema.resolveMaybeUndefValIntable(block, lhs_src, casted_lhs); const maybe_rhs_val = try sema.resolveMaybeUndefValIntable(block, rhs_src, casted_rhs); - if ((lhs_ty.tag() == .comptime_float and rhs_ty.tag() == .comptime_int) or - (lhs_ty.tag() == .comptime_int and rhs_ty.tag() == .comptime_float)) + if ((lhs_ty.zigTypeTag() == .ComptimeFloat and rhs_ty.zigTypeTag() == .ComptimeInt) or + (lhs_ty.zigTypeTag() == .ComptimeInt and rhs_ty.zigTypeTag() == .ComptimeFloat)) { // If it makes a difference whether we coerce to ints or floats before doing the division, error. // If lhs % rhs is 0, it doesn't matter. - var lhs_val = maybe_lhs_val orelse unreachable; - var rhs_val = maybe_rhs_val orelse unreachable; - var rem = lhs_val.floatRem(rhs_val, resolved_type, sema.arena, target) catch unreachable; - var float_rem = rem.toFloat(f32); - if (float_rem != 0.0) { - return sema.fail(block, src, "ambiguous coercion of division operands: '{s}' and '{s}': division has non-zero reminder: {d}", .{ - @tagName(lhs_ty.tag()), @tagName(rhs_ty.tag()), float_rem, + const lhs_val = maybe_lhs_val orelse unreachable; + const rhs_val = maybe_rhs_val orelse unreachable; + const rem = lhs_val.floatRem(rhs_val, resolved_type, sema.arena, target) catch unreachable; + if (rem.compareWithZero(.neq)) { + return sema.fail(block, src, "ambiguous coercion of division operands '{s}' and '{s}'; division has non-zero reminder '{}'", .{ + @tagName(lhs_ty.tag()), @tagName(rhs_ty.tag()), rem.fmtValue(resolved_type, sema.mod), }); } } diff --git a/test/cases/compile_errors/ambiguous_coercion_of_division_operands.zig b/test/cases/compile_errors/ambiguous_coercion_of_division_operands.zig new file mode 100644 index 0000000000..d8fa4e5726 --- /dev/null +++ b/test/cases/compile_errors/ambiguous_coercion_of_division_operands.zig @@ -0,0 +1,24 @@ +export fn entry1() void { + var f: f32 = 54.0 / 5; + _ = f; +} +export fn entry2() void { + var f: f32 = 54 / 5.0; + _ = f; +} +export fn entry3() void { + var f: f32 = 55.0 / 5; + _ = f; +} +export fn entry4() void { + var f: f32 = 55 / 5.0; + _ = f; +} + + +// error +// backend=stage2 +// target=native +// +// :2:23: error: ambiguous coercion of division operands 'comptime_float' and 'comptime_int'; division has non-zero reminder '4' +// :6:21: error: ambiguous coercion of division operands 'comptime_int' and 'comptime_float'; division has non-zero reminder '4' From 2cccd144914d8715894458f317fa4c3c572cdcad Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 17 Aug 2022 14:06:47 -0700 Subject: [PATCH 06/10] fix typo in compile error message --- src/Sema.zig | 2 +- .../ambiguous_coercion_of_division_operands.zig | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index 29a53ebb2a..5fd1c1ce4c 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -11211,7 +11211,7 @@ fn zirDiv(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins const rhs_val = maybe_rhs_val orelse unreachable; const rem = lhs_val.floatRem(rhs_val, resolved_type, sema.arena, target) catch unreachable; if (rem.compareWithZero(.neq)) { - return sema.fail(block, src, "ambiguous coercion of division operands '{s}' and '{s}'; division has non-zero reminder '{}'", .{ + return sema.fail(block, src, "ambiguous coercion of division operands '{s}' and '{s}'; non-zero remainder '{}'", .{ @tagName(lhs_ty.tag()), @tagName(rhs_ty.tag()), rem.fmtValue(resolved_type, sema.mod), }); } diff --git a/test/cases/compile_errors/ambiguous_coercion_of_division_operands.zig b/test/cases/compile_errors/ambiguous_coercion_of_division_operands.zig index d8fa4e5726..f3e51a1bed 100644 --- a/test/cases/compile_errors/ambiguous_coercion_of_division_operands.zig +++ b/test/cases/compile_errors/ambiguous_coercion_of_division_operands.zig @@ -15,10 +15,9 @@ export fn entry4() void { _ = f; } - // error // backend=stage2 // target=native // -// :2:23: error: ambiguous coercion of division operands 'comptime_float' and 'comptime_int'; division has non-zero reminder '4' -// :6:21: error: ambiguous coercion of division operands 'comptime_int' and 'comptime_float'; division has non-zero reminder '4' +// :2:23: error: ambiguous coercion of division operands 'comptime_float' and 'comptime_int'; non-zero remainder '4' +// :6:21: error: ambiguous coercion of division operands 'comptime_int' and 'comptime_float'; non-zero remainder '4' From 9b595dd55f1ae336e2fb4e2a64334efe137dc19a Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 17 Aug 2022 10:46:42 +0200 Subject: [PATCH 07/10] link-test: refactor and reorg Move common tests by target file format (Wasm, MachO) into helper functions in `link.zig`, and sort alphabetically within for easier tracking versus file organization on disk. --- test/link.zig | 97 ++++++++++++----------- test/link/macho/dead_strip/build.zig | 8 +- test/link/macho/pagezero/build.zig | 5 +- test/link/macho/search_strategy/build.zig | 8 +- test/link/macho/stack_size/build.zig | 3 +- test/link/macho/tls/build.zig | 5 +- 6 files changed, 69 insertions(+), 57 deletions(-) diff --git a/test/link.zig b/test/link.zig index a8a39a7018..88b370f343 100644 --- a/test/link.zig +++ b/test/link.zig @@ -23,7 +23,12 @@ pub fn addCases(cases: *tests.StandaloneContext) void { .build_modes = true, }); - cases.addBuildFile("test/link/wasm/type/build.zig", .{ + addWasmCases(cases); + addMachOCases(cases); +} + +fn addWasmCases(cases: *tests.StandaloneContext) void { + cases.addBuildFile("test/link/wasm/bss/build.zig", .{ .build_modes = true, .requires_stage2 = true, }); @@ -38,23 +43,13 @@ pub fn addCases(cases: *tests.StandaloneContext) void { .requires_stage2 = true, }); - cases.addBuildFile("test/link/wasm/bss/build.zig", .{ + cases.addBuildFile("test/link/wasm/type/build.zig", .{ .build_modes = true, .requires_stage2 = true, }); +} - cases.addBuildFile("test/link/macho/entry/build.zig", .{ - .build_modes = true, - }); - - cases.addBuildFile("test/link/macho/pagezero/build.zig", .{ - .build_modes = false, - }); - - cases.addBuildFile("test/link/macho/dylib/build.zig", .{ - .build_modes = true, - }); - +fn addMachOCases(cases: *tests.StandaloneContext) void { cases.addBuildFile("test/link/macho/dead_strip/build.zig", .{ .build_modes = false, }); @@ -64,41 +59,11 @@ pub fn addCases(cases: *tests.StandaloneContext) void { .requires_macos_sdk = true, }); - cases.addBuildFile("test/link/macho/needed_library/build.zig", .{ + cases.addBuildFile("test/link/macho/dylib/build.zig", .{ .build_modes = true, }); - cases.addBuildFile("test/link/macho/weak_library/build.zig", .{ - .build_modes = true, - }); - - cases.addBuildFile("test/link/macho/needed_framework/build.zig", .{ - .build_modes = true, - .requires_macos_sdk = true, - }); - - cases.addBuildFile("test/link/macho/weak_framework/build.zig", .{ - .build_modes = true, - .requires_macos_sdk = true, - }); - - // Try to build and run an Objective-C executable. - cases.addBuildFile("test/link/macho/objc/build.zig", .{ - .build_modes = true, - .requires_macos_sdk = true, - }); - - // Try to build and run an Objective-C++ executable. - cases.addBuildFile("test/link/macho/objcpp/build.zig", .{ - .build_modes = true, - .requires_macos_sdk = true, - }); - - cases.addBuildFile("test/link/macho/stack_size/build.zig", .{ - .build_modes = true, - }); - - cases.addBuildFile("test/link/macho/search_strategy/build.zig", .{ + cases.addBuildFile("test/link/macho/entry/build.zig", .{ .build_modes = true, }); @@ -107,7 +72,47 @@ pub fn addCases(cases: *tests.StandaloneContext) void { .requires_macos_sdk = true, }); + cases.addBuildFile("test/link/macho/needed_framework/build.zig", .{ + .build_modes = true, + .requires_macos_sdk = true, + }); + + cases.addBuildFile("test/link/macho/needed_library/build.zig", .{ + .build_modes = true, + }); + + cases.addBuildFile("test/link/macho/objc/build.zig", .{ + .build_modes = true, + .requires_macos_sdk = true, + }); + + cases.addBuildFile("test/link/macho/objcpp/build.zig", .{ + .build_modes = true, + .requires_macos_sdk = true, + }); + + cases.addBuildFile("test/link/macho/pagezero/build.zig", .{ + .build_modes = false, + }); + + cases.addBuildFile("test/link/macho/search_strategy/build.zig", .{ + .build_modes = true, + }); + + cases.addBuildFile("test/link/macho/stack_size/build.zig", .{ + .build_modes = true, + }); + cases.addBuildFile("test/link/macho/tls/build.zig", .{ .build_modes = true, }); + + cases.addBuildFile("test/link/macho/weak_library/build.zig", .{ + .build_modes = true, + }); + + cases.addBuildFile("test/link/macho/weak_framework/build.zig", .{ + .build_modes = true, + .requires_macos_sdk = true, + }); } diff --git a/test/link/macho/dead_strip/build.zig b/test/link/macho/dead_strip/build.zig index dea225dd6f..25759f5619 100644 --- a/test/link/macho/dead_strip/build.zig +++ b/test/link/macho/dead_strip/build.zig @@ -4,13 +4,14 @@ const LibExeObjectStep = std.build.LibExeObjStep; pub fn build(b: *Builder) void { const mode = b.standardReleaseOptions(); + const target: std.zig.CrossTarget = .{ .os_tag = .macos }; const test_step = b.step("test", "Test the program"); test_step.dependOn(b.getInstallStep()); { // Without -dead_strip, we expect `iAmUnused` symbol present - const exe = createScenario(b, mode); + const exe = createScenario(b, mode, target); const check = exe.checkObject(.macho); check.checkInSymtab(); @@ -23,7 +24,7 @@ pub fn build(b: *Builder) void { { // With -dead_strip, no `iAmUnused` symbol should be present - const exe = createScenario(b, mode); + const exe = createScenario(b, mode, target); exe.link_gc_sections = true; const check = exe.checkObject(.macho); @@ -36,10 +37,11 @@ pub fn build(b: *Builder) void { } } -fn createScenario(b: *Builder, mode: std.builtin.Mode) *LibExeObjectStep { +fn createScenario(b: *Builder, mode: std.builtin.Mode, target: std.zig.CrossTarget) *LibExeObjectStep { const exe = b.addExecutable("test", null); exe.addCSourceFile("main.c", &[0][]const u8{}); exe.setBuildMode(mode); + exe.setTarget(target); exe.linkLibC(); return exe; } diff --git a/test/link/macho/pagezero/build.zig b/test/link/macho/pagezero/build.zig index 9dbc0e6473..5a7044d960 100644 --- a/test/link/macho/pagezero/build.zig +++ b/test/link/macho/pagezero/build.zig @@ -3,13 +3,14 @@ const Builder = std.build.Builder; pub fn build(b: *Builder) void { const mode = b.standardReleaseOptions(); + const target: std.zig.CrossTarget = .{ .os_tag = .macos }; const test_step = b.step("test", "Test"); test_step.dependOn(b.getInstallStep()); { const exe = b.addExecutable("pagezero", null); - exe.setTarget(.{ .os_tag = .macos }); + exe.setTarget(target); exe.setBuildMode(mode); exe.addCSourceFile("main.c", &.{}); exe.linkLibC(); @@ -29,7 +30,7 @@ pub fn build(b: *Builder) void { { const exe = b.addExecutable("no_pagezero", null); - exe.setTarget(.{ .os_tag = .macos }); + exe.setTarget(target); exe.setBuildMode(mode); exe.addCSourceFile("main.c", &.{}); exe.linkLibC(); diff --git a/test/link/macho/search_strategy/build.zig b/test/link/macho/search_strategy/build.zig index 39a82bc6a7..e556b5bb23 100644 --- a/test/link/macho/search_strategy/build.zig +++ b/test/link/macho/search_strategy/build.zig @@ -1,17 +1,17 @@ const std = @import("std"); const Builder = std.build.Builder; const LibExeObjectStep = std.build.LibExeObjStep; -const target: std.zig.CrossTarget = .{ .os_tag = .macos }; pub fn build(b: *Builder) void { const mode = b.standardReleaseOptions(); + const target: std.zig.CrossTarget = .{ .os_tag = .macos }; const test_step = b.step("test", "Test"); test_step.dependOn(b.getInstallStep()); { // -search_dylibs_first - const exe = createScenario(b, mode); + const exe = createScenario(b, mode, target); exe.search_strategy = .dylibs_first; const check = exe.checkObject(.macho); @@ -26,7 +26,7 @@ pub fn build(b: *Builder) void { { // -search_paths_first - const exe = createScenario(b, mode); + const exe = createScenario(b, mode, target); exe.search_strategy = .paths_first; const run = std.build.EmulatableRunStep.create(b, "run", exe); @@ -36,7 +36,7 @@ pub fn build(b: *Builder) void { } } -fn createScenario(b: *Builder, mode: std.builtin.Mode) *LibExeObjectStep { +fn createScenario(b: *Builder, mode: std.builtin.Mode, target: std.zig.CrossTarget) *LibExeObjectStep { const static = b.addStaticLibrary("a", null); static.setTarget(target); static.setBuildMode(mode); diff --git a/test/link/macho/stack_size/build.zig b/test/link/macho/stack_size/build.zig index 3abf48df7a..91c44baf52 100644 --- a/test/link/macho/stack_size/build.zig +++ b/test/link/macho/stack_size/build.zig @@ -3,12 +3,13 @@ const Builder = std.build.Builder; pub fn build(b: *Builder) void { const mode = b.standardReleaseOptions(); + const target: std.zig.CrossTarget = .{ .os_tag = .macos }; const test_step = b.step("test", "Test"); test_step.dependOn(b.getInstallStep()); const exe = b.addExecutable("main", null); - exe.setTarget(.{ .os_tag = .macos }); + exe.setTarget(target); exe.setBuildMode(mode); exe.addCSourceFile("main.c", &.{}); exe.linkLibC(); diff --git a/test/link/macho/tls/build.zig b/test/link/macho/tls/build.zig index 7bf3ea1c9e..031a05cedf 100644 --- a/test/link/macho/tls/build.zig +++ b/test/link/macho/tls/build.zig @@ -1,4 +1,5 @@ -const Builder = @import("std").build.Builder; +const std = @import("std"); +const Builder = std.build.Builder; pub fn build(b: *Builder) void { const mode = b.standardReleaseOptions(); @@ -6,11 +7,13 @@ pub fn build(b: *Builder) void { const lib = b.addSharedLibrary("a", null, b.version(1, 0, 0)); lib.setBuildMode(mode); + lib.setTarget(target); lib.addCSourceFile("a.c", &.{}); lib.linkLibC(); const test_exe = b.addTest("main.zig"); test_exe.setBuildMode(mode); + test_exe.setTarget(target); test_exe.linkLibrary(lib); test_exe.linkLibC(); From 515ee5b2fa0161e3df4cfe47c71847f8b3476bf2 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 17 Aug 2022 13:30:23 +0200 Subject: [PATCH 08/10] libstd: do not follow symlinks in renameatW This correctly mimicks POSIX behavior. --- lib/std/os.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/std/os.zig b/lib/std/os.zig index 984758565c..c757561b07 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -2651,6 +2651,7 @@ pub fn renameatW( .creation = windows.FILE_OPEN, .io_mode = .blocking, .filter = .any, // This function is supposed to rename both files and directories. + .follow_symlinks = false, }) catch |err| switch (err) { error.WouldBlock => unreachable, // Not possible without `.share_access_nonblocking = true`. else => |e| return e, From b6ce0cce69e4bf37509f99040abda4ca92bec6cb Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 17 Aug 2022 14:53:20 +0200 Subject: [PATCH 09/10] windows-ci: pass -Domit-stage2 to test-toolchain --- ci/azure/pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/azure/pipelines.yml b/ci/azure/pipelines.yml index 5ff0a5d61b..5d208b31a2 100644 --- a/ci/azure/pipelines.yml +++ b/ci/azure/pipelines.yml @@ -103,7 +103,7 @@ jobs: #& "$ZIGINSTALLDIR\bin\zig.exe" test "..\test\behavior.zig" -fno-stage1 -fLLVM -I "..\test" 2>&1 #CheckLastExitCode - & "$ZIGINSTALLDIR\bin\zig.exe" build test-toolchain -Dskip-non-native -Dskip-stage2-tests 2>&1 + & "$ZIGINSTALLDIR\bin\zig.exe" build test-toolchain -Dskip-non-native -Dskip-stage2-tests -Domit-stage2 2>&1 CheckLastExitCode & "$ZIGINSTALLDIR\bin\zig.exe" build test-std -Dskip-non-native 2>&1 CheckLastExitCode From 79757f233d9bfc646caa13d20243266a19bbdf91 Mon Sep 17 00:00:00 2001 From: Techatrix <19954306+Techatrix@users.noreply.github.com> Date: Wed, 17 Aug 2022 21:28:11 +0200 Subject: [PATCH 10/10] fix memory leak in NativePaths.zig --- lib/std/zig/system/NativePaths.zig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/std/zig/system/NativePaths.zig b/lib/std/zig/system/NativePaths.zig index e9e7460314..5f52b04ce4 100644 --- a/lib/std/zig/system/NativePaths.zig +++ b/lib/std/zig/system/NativePaths.zig @@ -109,6 +109,8 @@ pub fn detect(allocator: Allocator, native_info: NativeTargetInfo) !NativePaths if (native_target.os.tag != .windows) { const triple = try native_target.linuxTriple(allocator); + defer allocator.free(triple); + const qual = native_target.cpu.arch.ptrBitWidth(); // TODO: $ ld --verbose | grep SEARCH_DIR