authorgravatar for mail@isaacfreund.comIsaac Freund <mail@isaacfreund.com> 2023-01-22 16:40:00+01:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2023-01-29 15:07:06-05:00
log23b7d28896609e3f01765730599119baf53a56c9
treecebad2d68d4c0a3d7059f181af6c2b7e1fff0116
parent7c2ba950a758b86893bfbe73521b29895f7ac4f0

std: restrict mem.span() and mem.len() to sentinel terminated pointers

These functions are currently footgunny when working with pointers to arrays and slices. They just return the stated length of the array/slice without iterating and looking for the first sentinel, even if the array/slice is a sentinel terminated type. From looking at the quite small list of places in the standard library/compiler that this change breaks existing code, the new code looks to be more readable in all cases. The usage of std.mem.span/len was totally unneeded in most of the cases affected by this breaking change. We could remove these functions entirely in favor of other existing functions in std.mem such as std.mem.sliceTo(), but that would be a somewhat nasty breaking change as std.mem.span() is very widely used for converting sentinel terminated pointers to slices. It is however not at all widely used for anything else. Therefore I think it is better to break these few non-standard and potentially incorrect usages of these functions now and at some later time, if deemed worthwhile, finally remove these functions. If we wait for at least a full release cycle so that everyone adapts to this change first, updating for the removal could be a simple find and replace without needing to worry about the semantics.

9 files changed, 56 insertions(+), 94 deletions(-)

lib/std/Thread.zig+1-1
...@@ -166,7 +166,7 @@ pub const GetNameError = error{...@@ -166,7 +166,7 @@ pub const GetNameError = error{
166166
167pub fn getName(self: Thread, buffer_ptr: *[max_name_len:0]u8) GetNameError!?[]const u8 {167pub fn getName(self: Thread, buffer_ptr: *[max_name_len:0]u8) GetNameError!?[]const u8 {
168 buffer_ptr[max_name_len] = 0;168 buffer_ptr[max_name_len] = 0;
169 var buffer = std.mem.span(buffer_ptr);169 var buffer: [:0]u8 = buffer_ptr;
170170
171 switch (target.os.tag) {171 switch (target.os.tag) {
172 .linux => if (use_pthreads and is_gnu) {172 .linux => if (use_pthreads and is_gnu) {
lib/std/bounded_array.zig+5-1
...@@ -29,7 +29,11 @@ pub fn BoundedArray(comptime T: type, comptime buffer_capacity: usize) type {...@@ -29,7 +29,11 @@ pub fn BoundedArray(comptime T: type, comptime buffer_capacity: usize) type {
29 }29 }
3030
31 /// View the internal array as a slice whose size was previously set.31 /// View the internal array as a slice whose size was previously set.
32 pub fn slice(self: anytype) mem.Span(@TypeOf(&self.buffer)) {32 pub fn slice(self: anytype) switch (@TypeOf(&self.buffer)) {
33 *[buffer_capacity]T => []T,
34 *const [buffer_capacity]T => []const T,
35 else => unreachable,
36 } {
33 return self.buffer[0..self.len];37 return self.buffer[0..self.len];
34 }38 }
3539
lib/std/cstr.zig-1
...@@ -28,7 +28,6 @@ test "cstr fns" {...@@ -28,7 +28,6 @@ test "cstr fns" {
2828
29fn testCStrFnsImpl() !void {29fn testCStrFnsImpl() !void {
30 try testing.expect(cmp("aoeu", "aoez") == -1);30 try testing.expect(cmp("aoeu", "aoez") == -1);
31 try testing.expect(mem.len("123456789") == 9);
32}31}
3332
34/// Returns a mutable, null-terminated slice with the same length as `slice`.33/// Returns a mutable, null-terminated slice with the same length as `slice`.
lib/std/fs.zig+1-1
...@@ -834,7 +834,7 @@ pub const IterableDir = struct {...@@ -834,7 +834,7 @@ pub const IterableDir = struct {
834 self.end_index = self.index; // Force fd_readdir in the next loop.834 self.end_index = self.index; // Force fd_readdir in the next loop.
835 continue :start_over;835 continue :start_over;
836 }836 }
837 const name = mem.span(self.buf[name_index .. name_index + entry.d_namlen]);837 const name = self.buf[name_index .. name_index + entry.d_namlen];
838838
839 const next_index = name_index + entry.d_namlen;839 const next_index = name_index + entry.d_namlen;
840 self.index = next_index;840 self.index = next_index;
lib/std/io/fixed_buffer_stream.zig+19-6
...@@ -113,14 +113,27 @@ pub fn FixedBufferStream(comptime Buffer: type) type {...@@ -113,14 +113,27 @@ pub fn FixedBufferStream(comptime Buffer: type) type {
113 };113 };
114}114}
115115
116pub fn fixedBufferStream(buffer: anytype) FixedBufferStream(NonSentinelSpan(@TypeOf(buffer))) {116pub fn fixedBufferStream(buffer: anytype) FixedBufferStream(Slice(@TypeOf(buffer))) {
117 return .{ .buffer = mem.span(buffer), .pos = 0 };117 return .{ .buffer = buffer, .pos = 0 };
118}118}
119119
120fn NonSentinelSpan(comptime T: type) type {120fn Slice(comptime T: type) type {
121 var ptr_info = @typeInfo(mem.Span(T)).Pointer;121 switch (@typeInfo(T)) {
122 ptr_info.sentinel = null;122 .Pointer => |ptr_info| {
123 return @Type(.{ .Pointer = ptr_info });123 var new_ptr_info = ptr_info;
124 switch (ptr_info.size) {
125 .Slice => {},
126 .One => switch (@typeInfo(ptr_info.child)) {
127 .Array => |info| new_ptr_info.child = info.child,
128 else => @compileError("invalid type given to fixedBufferStream"),
129 },
130 else => @compileError("invalid type given to fixedBufferStream"),
131 }
132 new_ptr_info.size = .Slice;
133 return @Type(.{ .Pointer = new_ptr_info });
134 },
135 else => @compileError("invalid type given to fixedBufferStream"),
136 }
124}137}
125138
126test "FixedBufferStream output" {139test "FixedBufferStream output" {
lib/std/mem.zig+24-78
...@@ -636,12 +636,9 @@ test "indexOfDiff" {...@@ -636,12 +636,9 @@ test "indexOfDiff" {
636 try testing.expectEqual(indexOfDiff(u8, "xne", "one"), 0);636 try testing.expectEqual(indexOfDiff(u8, "xne", "one"), 0);
637}637}
638638
639/// Takes a pointer to an array, a sentinel-terminated pointer, or a slice, and639/// Takes a sentinel-terminated pointer and returns a slice preserving pointer attributes.
640/// returns a slice. If there is a sentinel on the input type, there will be a640/// `[*c]` pointers are assumed to be 0-terminated and assumed to not be allowzero.
641/// sentinel on the output type. The constness of the output type matches641fn Span(comptime T: type) type {
642/// the constness of the input type. `[*c]` pointers are assumed to be 0-terminated,
643/// and assumed to not allow null.
644pub fn Span(comptime T: type) type {
645 switch (@typeInfo(T)) {642 switch (@typeInfo(T)) {
646 .Optional => |optional_info| {643 .Optional => |optional_info| {
647 return ?Span(optional_info.child);644 return ?Span(optional_info.child);
...@@ -649,39 +646,22 @@ pub fn Span(comptime T: type) type {...@@ -649,39 +646,22 @@ pub fn Span(comptime T: type) type {
649 .Pointer => |ptr_info| {646 .Pointer => |ptr_info| {
650 var new_ptr_info = ptr_info;647 var new_ptr_info = ptr_info;
651 switch (ptr_info.size) {648 switch (ptr_info.size) {
652 .One => switch (@typeInfo(ptr_info.child)) {
653 .Array => |info| {
654 new_ptr_info.child = info.child;
655 new_ptr_info.sentinel = info.sentinel;
656 },
657 else => @compileError("invalid type given to std.mem.Span"),
658 },
659 .C => {649 .C => {
660 new_ptr_info.sentinel = &@as(ptr_info.child, 0);650 new_ptr_info.sentinel = &@as(ptr_info.child, 0);
661 new_ptr_info.is_allowzero = false;651 new_ptr_info.is_allowzero = false;
662 },652 },
663 .Many, .Slice => {},653 .Many => if (ptr_info.sentinel == null) @compileError("invalid type given to std.mem.span: " ++ @typeName(T)),
654 .One, .Slice => @compileError("invalid type given to std.mem.span: " ++ @typeName(T)),
664 }655 }
665 new_ptr_info.size = .Slice;656 new_ptr_info.size = .Slice;
666 return @Type(.{ .Pointer = new_ptr_info });657 return @Type(.{ .Pointer = new_ptr_info });
667 },658 },
668 else => @compileError("invalid type given to std.mem.Span"),659 else => {},
669 }660 }
661 @compileError("invalid type given to std.mem.span: " ++ @typeName(T));
670}662}
671663
672test "Span" {664test "Span" {
673 try testing.expect(Span(*[5]u16) == []u16);
674 try testing.expect(Span(?*[5]u16) == ?[]u16);
675 try testing.expect(Span(*const [5]u16) == []const u16);
676 try testing.expect(Span(?*const [5]u16) == ?[]const u16);
677 try testing.expect(Span([]u16) == []u16);
678 try testing.expect(Span(?[]u16) == ?[]u16);
679 try testing.expect(Span([]const u8) == []const u8);
680 try testing.expect(Span(?[]const u8) == ?[]const u8);
681 try testing.expect(Span([:1]u16) == [:1]u16);
682 try testing.expect(Span(?[:1]u16) == ?[:1]u16);
683 try testing.expect(Span([:1]const u8) == [:1]const u8);
684 try testing.expect(Span(?[:1]const u8) == ?[:1]const u8);
685 try testing.expect(Span([*:1]u16) == [:1]u16);665 try testing.expect(Span([*:1]u16) == [:1]u16);
686 try testing.expect(Span(?[*:1]u16) == ?[:1]u16);666 try testing.expect(Span(?[*:1]u16) == ?[:1]u16);
687 try testing.expect(Span([*:1]const u8) == [:1]const u8);667 try testing.expect(Span([*:1]const u8) == [:1]const u8);
...@@ -692,13 +672,10 @@ test "Span" {...@@ -692,13 +672,10 @@ test "Span" {
692 try testing.expect(Span(?[*c]const u8) == ?[:0]const u8);672 try testing.expect(Span(?[*c]const u8) == ?[:0]const u8);
693}673}
694674
695/// Takes a pointer to an array, a sentinel-terminated pointer, or a slice, and675/// Takes a sentinel-terminated pointer and returns a slice, iterating over the
696/// returns a slice. If there is a sentinel on the input type, there will be a676/// memory to find the sentinel and determine the length.
697/// sentinel on the output type. The constness of the output type matches677/// Ponter attributes such as const are preserved.
698/// the constness of the input type.678/// `[*c]` pointers are assumed to be non-null and 0-terminated.
699///
700/// When there is both a sentinel and an array length or slice length, the
701/// length value is used instead of the sentinel.
702pub fn span(ptr: anytype) Span(@TypeOf(ptr)) {679pub fn span(ptr: anytype) Span(@TypeOf(ptr)) {
703 if (@typeInfo(@TypeOf(ptr)) == .Optional) {680 if (@typeInfo(@TypeOf(ptr)) == .Optional) {
704 if (ptr) |non_null| {681 if (ptr) |non_null| {
...@@ -722,7 +699,6 @@ test "span" {...@@ -722,7 +699,6 @@ test "span" {
722 var array: [5]u16 = [_]u16{ 1, 2, 3, 4, 5 };699 var array: [5]u16 = [_]u16{ 1, 2, 3, 4, 5 };
723 const ptr = @as([*:3]u16, array[0..2 :3]);700 const ptr = @as([*:3]u16, array[0..2 :3]);
724 try testing.expect(eql(u16, span(ptr), &[_]u16{ 1, 2 }));701 try testing.expect(eql(u16, span(ptr), &[_]u16{ 1, 2 }));
725 try testing.expect(eql(u16, span(&array), &[_]u16{ 1, 2, 3, 4, 5 }));
726 try testing.expectEqual(@as(?[:0]u16, null), span(@as(?[*:0]u16, null)));702 try testing.expectEqual(@as(?[:0]u16, null), span(@as(?[*:0]u16, null)));
727}703}
728704
...@@ -919,22 +895,15 @@ test "lenSliceTo" {...@@ -919,22 +895,15 @@ test "lenSliceTo" {
919 }895 }
920}896}
921897
922/// Takes a pointer to an array, an array, a vector, a sentinel-terminated pointer,898/// Takes a sentinel-terminated pointer and iterates over the memory to find the
923/// a slice or a tuple, and returns the length.899/// sentinel and determine the length.
924/// In the case of a sentinel-terminated array, it uses the array length.900/// `[*c]` pointers are assumed to be non-null and 0-terminated.
925/// For C pointers it assumes it is a pointer-to-many with a 0 sentinel.
926pub fn len(value: anytype) usize {901pub fn len(value: anytype) usize {
927 return switch (@typeInfo(@TypeOf(value))) {902 switch (@typeInfo(@TypeOf(value))) {
928 .Array => |info| info.len,
929 .Vector => |info| info.len,
930 .Pointer => |info| switch (info.size) {903 .Pointer => |info| switch (info.size) {
931 .One => switch (@typeInfo(info.child)) {
932 .Array => value.len,
933 else => @compileError("invalid type given to std.mem.len"),
934 },
935 .Many => {904 .Many => {
936 const sentinel_ptr = info.sentinel orelse905 const sentinel_ptr = info.sentinel orelse
937 @compileError("length of pointer with no sentinel");906 @compileError("invalid type given to std.mem.len: " ++ @typeName(@TypeOf(value)));
938 const sentinel = @ptrCast(*align(1) const info.child, sentinel_ptr).*;907 const sentinel = @ptrCast(*align(1) const info.child, sentinel_ptr).*;
939 return indexOfSentinel(info.child, sentinel, value);908 return indexOfSentinel(info.child, sentinel, value);
940 },909 },
...@@ -942,41 +911,18 @@ pub fn len(value: anytype) usize {...@@ -942,41 +911,18 @@ pub fn len(value: anytype) usize {
942 assert(value != null);911 assert(value != null);
943 return indexOfSentinel(info.child, 0, value);912 return indexOfSentinel(info.child, 0, value);
944 },913 },
945 .Slice => value.len,914 else => @compileError("invalid type given to std.mem.len: " ++ @typeName(@TypeOf(value))),
946 },915 },
947 .Struct => |info| if (info.is_tuple) {916 else => @compileError("invalid type given to std.mem.len: " ++ @typeName(@TypeOf(value))),
948 return info.fields.len;917 }
949 } else @compileError("invalid type given to std.mem.len"),
950 else => @compileError("invalid type given to std.mem.len"),
951 };
952}918}
953919
954test "len" {920test "len" {
955 try testing.expect(len("aoeu") == 4);921 var array: [5]u16 = [_]u16{ 1, 2, 0, 4, 5 };
956922 const ptr = @as([*:4]u16, array[0..3 :4]);
957 {923 try testing.expect(len(ptr) == 3);
958 var array: [5]u16 = [_]u16{ 1, 2, 3, 4, 5 };924 const c_ptr = @as([*c]u16, ptr);
959 try testing.expect(len(&array) == 5);925 try testing.expect(len(c_ptr) == 2);
960 try testing.expect(len(array[0..3]) == 3);
961 array[2] = 0;
962 const ptr = @as([*:0]u16, array[0..2 :0]);
963 try testing.expect(len(ptr) == 2);
964 }
965 {
966 var array: [5:0]u16 = [_:0]u16{ 1, 2, 3, 4, 5 };
967 try testing.expect(len(&array) == 5);
968 array[2] = 0;
969 try testing.expect(len(&array) == 5);
970 }
971 {
972 const vector: meta.Vector(2, u32) = [2]u32{ 1, 2 };
973 try testing.expect(len(vector) == 2);
974 }
975 {
976 const tuple = .{ 1, 2 };
977 try testing.expect(len(tuple) == 2);
978 try testing.expect(tuple[0] == 1);
979 }
980}926}
981927
982pub fn indexOfSentinel(comptime Elem: type, comptime sentinel: Elem, ptr: [*:sentinel]const Elem) usize {928pub fn indexOfSentinel(comptime Elem: type, comptime sentinel: Elem, ptr: [*:sentinel]const Elem) usize {
src/link/MachO/load_commands.zig+1-1
...@@ -12,7 +12,7 @@ pub const default_dyld_path: [*:0]const u8 = "/usr/lib/dyld";...@@ -12,7 +12,7 @@ pub const default_dyld_path: [*:0]const u8 = "/usr/lib/dyld";
1212
13fn calcInstallNameLen(cmd_size: u64, name: []const u8, assume_max_path_len: bool) u64 {13fn calcInstallNameLen(cmd_size: u64, name: []const u8, assume_max_path_len: bool) u64 {
14 const darwin_path_max = 1024;14 const darwin_path_max = 1024;
15 const name_len = if (assume_max_path_len) darwin_path_max else std.mem.len(name) + 1;15 const name_len = if (assume_max_path_len) darwin_path_max else name.len + 1;
16 return mem.alignForwardGeneric(u64, cmd_size + name_len, @alignOf(u64));16 return mem.alignForwardGeneric(u64, cmd_size + name_len, @alignOf(u64));
17}17}
1818
src/main.zig+4-4
...@@ -893,7 +893,7 @@ fn buildOutputType(...@@ -893,7 +893,7 @@ fn buildOutputType(
893 i: usize = 0,893 i: usize = 0,
894 fn next(it: *@This()) ?[]const u8 {894 fn next(it: *@This()) ?[]const u8 {
895 if (it.i >= it.args.len) {895 if (it.i >= it.args.len) {
896 if (it.resp_file) |*resp| return if (resp.next()) |sentinel| std.mem.span(sentinel) else null;896 if (it.resp_file) |*resp| return resp.next();
897 return null;897 return null;
898 }898 }
899 defer it.i += 1;899 defer it.i += 1;
...@@ -901,7 +901,7 @@ fn buildOutputType(...@@ -901,7 +901,7 @@ fn buildOutputType(
901 }901 }
902 fn nextOrFatal(it: *@This()) []const u8 {902 fn nextOrFatal(it: *@This()) []const u8 {
903 if (it.i >= it.args.len) {903 if (it.i >= it.args.len) {
904 if (it.resp_file) |*resp| if (resp.next()) |sentinel| return std.mem.span(sentinel);904 if (it.resp_file) |*resp| if (resp.next()) |ret| return ret;
905 fatal("expected parameter after {s}", .{it.args[it.i - 1]});905 fatal("expected parameter after {s}", .{it.args[it.i - 1]});
906 }906 }
907 defer it.i += 1;907 defer it.i += 1;
...@@ -4973,7 +4973,7 @@ pub const ClangArgIterator = struct {...@@ -4973,7 +4973,7 @@ pub const ClangArgIterator = struct {
4973 // rather than an argument to a parameter.4973 // rather than an argument to a parameter.
4974 // We adjust the len below when necessary.4974 // We adjust the len below when necessary.
4975 self.other_args = (self.argv.ptr + self.next_index)[0..1];4975 self.other_args = (self.argv.ptr + self.next_index)[0..1];
4976 var arg = mem.span(self.argv[self.next_index]);4976 var arg = self.argv[self.next_index];
4977 self.incrementArgIndex();4977 self.incrementArgIndex();
49784978
4979 if (mem.startsWith(u8, arg, "@")) {4979 if (mem.startsWith(u8, arg, "@")) {
...@@ -5017,7 +5017,7 @@ pub const ClangArgIterator = struct {...@@ -5017,7 +5017,7 @@ pub const ClangArgIterator = struct {
50175017
5018 self.has_next = true;5018 self.has_next = true;
5019 self.other_args = (self.argv.ptr + self.next_index)[0..1]; // We adjust len below when necessary.5019 self.other_args = (self.argv.ptr + self.next_index)[0..1]; // We adjust len below when necessary.
5020 arg = mem.span(self.argv[self.next_index]);5020 arg = self.argv[self.next_index];
5021 self.incrementArgIndex();5021 self.incrementArgIndex();
5022 }5022 }
50235023
test/behavior/basic.zig+1-1
...@@ -703,7 +703,7 @@ test "string concatenation" {...@@ -703,7 +703,7 @@ test "string concatenation" {
703 comptime try expect(@TypeOf(a) == *const [12:0]u8);703 comptime try expect(@TypeOf(a) == *const [12:0]u8);
704 comptime try expect(@TypeOf(b) == *const [12:0]u8);704 comptime try expect(@TypeOf(b) == *const [12:0]u8);
705705
706 const len = mem.len(b);706 const len = b.len;
707 const len_with_null = len + 1;707 const len_with_null = len + 1;
708 {708 {
709 var i: u32 = 0;709 var i: u32 = 0;