ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
33.45k stars 2.44k forks source link

safety check for setting the wrong sentinel value #9791

Open donaldcallen opened 2 years ago

donaldcallen commented 2 years ago
const std = @import("std");
const expect = std.testing.expect;

test "null terminated array" {
    var array:[4:0]u8 = undefined;
    array = [4:0]u8 {1, 2, 3, 4};
    array[4] = 1;

    try expect(@TypeOf(array) == [4:0]u8);
    try expect(array.len == 4);
    try expect(array[4] == 1);
}

If I declare an array to be sentinal-terminated, which says in the type what the sentinal is, I'd suggest that the compiler generate code, at least in Debug mode, to check settings of the sentinal to be sure they are as specified in the type, when the lv access is compile-time known, as it is here. I would have expected the above test to fail and it does not.

andrewrk commented 2 years ago

Expected behavior for this particular test case would be a compile error since the index and the element value being set are both comptime-known.

If the element index is comptime known to be the length, but the element value is runtime known, then there should be a runtime safety check to make sure the element value is equal to the sentinel.

If both are runtime known, should there be a safety check? Maybe. On the other hand this could be quite expensive.

mtlynch commented 8 months ago

This behavior seems to have changed since this issue was first filed.

As of Zig 0.11.0, this code compiles and passes testing:

const std = @import("std");
const expectEqual = std.testing.expectEqual;

test "null terminated array" {
    var array: [4:0]u8 = undefined;
    array = [4:0]u8{ 1, 2, 3, 4 };
    array[4] = 1; // Zig effectively ignores this line.

    try expectEqual(@TypeOf(array), [4:0]u8);
    try expectEqual(4, array.len);
    try expectEqual(0, array[4]);
}

It's surprising that Zig allows the code to attempt to reassign the slot in the array with the sentinel value, but Zig seems to just ignore the assignment and preserve the sentinel.

This is better than allowing the sentinel to be overwritten, but it's still surprising that this doesn't result in a compile-time or runtime error.

andrewrk commented 8 months ago

it's ... surprising that this doesn't result in a compile-time or runtime error.

To be clear, that's exactly what this issue is proposing to do - make this Illegal Behavior, thus resulting in a compile error if caught at compile-time, or otherwise caught at runtime in safe optimization modes.

The only reason not to accept it, in my opinion, would be if the safety check could not be implemented efficiently.