Hello everyone, Zig newcomer here!
I was playing with enums module, specifically with enums.directEnumArray and enums.directEnumArrayDefault - and while they are awesome, I found part of implementation “problematic”.
Here is the implementation C/P from the std source
/// Initializes an array of Data which can be indexed by
/// @intCast(usize, @intFromEnum(enum_value)).
/// If the enum is non-exhaustive, the resulting array will only be large enough
/// to hold all explicit fields.
/// If the enum contains any fields with values that cannot be represented
/// by usize, a compile error is issued. The max_unused_slots parameter limits
/// the total number of items which have no matching enum key (holes in the enum
/// numbering). So for example, if an enum has values 1, 2, 5, and 6, max_unused_slots
/// must be at least 3, to allow unused slots 0, 3, and 4.
/// The init_values parameter must be a struct with field names that match the enum values.
/// If the enum has multiple fields with the same value, the name of the first one must
/// be used.
pub fn directEnumArray(
comptime E: type,
comptime Data: type,
comptime max_unused_slots: comptime_int,
init_values: EnumFieldStruct(E, Data, null),
) [directEnumArrayLen(E, max_unused_slots)]Data {
return directEnumArrayDefault(E, Data, null, max_unused_slots, init_values);
}
test directEnumArray {
const E = enum(i4) { a = 4, b = 6, c = 2 };
var runtime_false: bool = false;
_ = &runtime_false;
const array = directEnumArray(E, bool, 4, .{
.a = true,
.b = runtime_false,
.c = true,
});
try testing.expectEqual([7]bool, @TypeOf(array));
try testing.expectEqual(true, array[4]);
try testing.expectEqual(false, array[6]);
try testing.expectEqual(true, array[2]);
}
/// Initializes an array of Data which can be indexed by
/// @intCast(usize, @intFromEnum(enum_value)). The enum must be exhaustive.
/// If the enum contains any fields with values that cannot be represented
/// by usize, a compile error is issued. The max_unused_slots parameter limits
/// the total number of items which have no matching enum key (holes in the enum
/// numbering). So for example, if an enum has values 1, 2, 5, and 6, max_unused_slots
/// must be at least 3, to allow unused slots 0, 3, and 4.
/// The init_values parameter must be a struct with field names that match the enum values.
/// If the enum has multiple fields with the same value, the name of the first one must
/// be used.
pub fn directEnumArrayDefault(
comptime E: type,
comptime Data: type,
comptime default: ?Data,
comptime max_unused_slots: comptime_int,
init_values: EnumFieldStruct(E, Data, default),
) [directEnumArrayLen(E, max_unused_slots)]Data {
const len = comptime directEnumArrayLen(E, max_unused_slots);
var result: [len]Data = if (default) |d| [_]Data{d} ** len else undefined;
inline for (@typeInfo(@TypeOf(init_values)).@"struct".fields) |f| {
const enum_value = @field(E, f.name);
const index = @as(usize, @intCast(@intFromEnum(enum_value)));
result[index] = @field(init_values, f.name);
}
return result;
}
test directEnumArrayDefault {
const E = enum(i4) { a = 4, b = 6, c = 2 };
var runtime_false: bool = false;
_ = &runtime_false;
const array = directEnumArrayDefault(E, bool, false, 4, .{
.a = true,
.b = runtime_false,
});
try testing.expectEqual([7]bool, @TypeOf(array));
try testing.expectEqual(true, array[4]);
try testing.expectEqual(false, array[6]);
try testing.expectEqual(false, array[2]);
}
The “problematic” part I was referring to is this line:
var result: [len]Data = if (default) |d| [_]Data{d} ** len else undefined; in directEnumArrayDefault.
IMHO, the better function design would be to require users to explicitly pass the fallback/default value be it undefined or null, or smth. else.
I think this undefined is too “hidden” and can easily lead to UB, especially if one calls directEnumArray where we have a layer of indirection - in fact I’d probably remove directEnumArray altogether in this case.
Also being able to pass either null or undefined, IMHO makes code bit less “weird” (link).
I think that if function design change is not acceptable, at least documenting this behaviour should be acceptable/desirable 