From a320a795bc0879edf4cab778663ddc189a19563f Mon Sep 17 00:00:00 2001 From: kayomn Date: Sat, 22 Jul 2023 13:57:39 +0100 Subject: [PATCH] Amend code review issues --- source/coral/arena.zig | 4 ++-- source/coral/io.zig | 4 ++-- source/coral/list.zig | 16 ++-------------- source/coral/map.zig | 21 ++++++++++++--------- source/coral/math.zig | 25 +++++++++++++++++++------ source/coral/utf8.zig | 2 +- source/ona/app.zig | 34 +++++++++++++++++++++++++--------- source/ona/kym.zig | 2 +- source/ona/kym/Ast.zig | 10 +++++----- source/ona/kym/Table.zig | 2 +- source/ona/kym/tokens.zig | 26 +++++++++++++------------- source/ona/ona.zig | 2 +- source/runner.zig | 2 +- 13 files changed, 85 insertions(+), 65 deletions(-) diff --git a/source/coral/arena.zig b/source/coral/arena.zig index 3600a0d..7ba2b1d 100644 --- a/source/coral/arena.zig +++ b/source/coral/arena.zig @@ -74,7 +74,7 @@ pub const Stacking = struct { const aligned_size = (size + alignment - 1) & ~(alignment - 1); if (self.pages.values.len == 0) { - const page = try self.allocate_page(math.max(self.min_page_size, aligned_size)); + const page = try self.allocate_page(@max(self.min_page_size, aligned_size)); page.used = size; @@ -84,7 +84,7 @@ pub const Stacking = struct { var page = self.current_page() orelse unreachable; if (page.available() <= aligned_size) { - page = try self.allocate_page(math.max(self.min_page_size, aligned_size)); + page = try self.allocate_page(@max(self.min_page_size, aligned_size)); } debug.assert(page.available() >= size); diff --git a/source/coral/io.zig b/source/coral/io.zig index 8882160..ad969c9 100644 --- a/source/coral/io.zig +++ b/source/coral/io.zig @@ -96,7 +96,7 @@ pub const FixedBuffer = struct { } pub fn write(self: *FixedBuffer, bytes: []const Byte) usize { - const writable = math.min(self.bytes.len, bytes.len); + const writable = @min(self.bytes.len, bytes.len); copy(self.bytes, bytes); @@ -251,7 +251,7 @@ pub fn copy(target: []Byte, source: []const Byte) void { } pub fn compare(this: []const Byte, that: []const Byte) isize { - const range = math.min(this.len, that.len); + const range = @min(this.len, that.len); var index: usize = 0; while (index < range) : (index += 1) { diff --git a/source/coral/list.zig b/source/coral/list.zig index a265cb5..f6915c9 100644 --- a/source/coral/list.zig +++ b/source/coral/list.zig @@ -16,16 +16,6 @@ pub fn Stack(comptime Value: type) type { self.values = self.values[0 .. 0]; } - pub fn drop(self: *Self, amount: usize) bool { - if (amount > self.values.len) { - return false; - } - - self.values = self.values[0 .. self.values.len - amount]; - - return true; - } - pub fn free(self: *Self) void { if (self.capacity == 0) { return; @@ -111,7 +101,7 @@ pub fn Stack(comptime Value: type) type { pub fn push_one(self: *Self, value: Value) io.AllocationError!void { if (self.values.len == self.capacity) { - try self.grow(math.max(1, self.capacity)); + try self.grow(@max(1, self.capacity)); } const offset_index = self.values.len; @@ -128,7 +118,5 @@ pub fn stack_as_writer(self: *ByteStack) io.Writer { } fn write_stack(stack: *ByteStack, bytes: []const io.Byte) ?usize { - stack.push_all(bytes) catch return null; - - return bytes.len; + return stack.push_all(bytes) catch null; } diff --git a/source/coral/map.zig b/source/coral/map.zig index f9c961f..1bc6a96 100644 --- a/source/coral/map.zig +++ b/source/coral/map.zig @@ -112,6 +112,9 @@ pub fn StringTable(comptime Value: type) type { pub fn Table(comptime Key: type, comptime Value: type, comptime traits: TableTraits(Key)) type { const load_max = 0.75; + const hash_int = @typeInfo(usize).Int; + const max_int = math.max_int(hash_int); + const min_int = math.min_int(hash_int); return struct { allocator: io.Allocator, @@ -123,8 +126,8 @@ pub fn Table(comptime Key: type, comptime Value: type, comptime traits: TableTra value: Value, fn write_into(self: Entry, entry_table: []?Entry) bool { - const hash_max = math.min(math.max_int(@typeInfo(usize).Int), entry_table.len); - var hashed_key = math.wrap(traits.hash(self.key), math.min_int(@typeInfo(usize).Int), hash_max); + const hash_max = @min(max_int, entry_table.len); + var hashed_key = math.wrap(traits.hash(self.key), min_int, hash_max); var iterations = @as(usize, 0); while (true) : (iterations += 1) { @@ -175,8 +178,8 @@ pub fn Table(comptime Key: type, comptime Value: type, comptime traits: TableTra } pub fn remove(self: *Self, key: Key) ?Entry { - const hash_max = math.min(math.max_int(@typeInfo(usize).Int), self.entries.len); - var hashed_key = math.wrap(traits.hash(key), math.min_int(@typeInfo(usize).Int), hash_max); + const hash_max = @min(max_int, self.entries.len); + var hashed_key = math.wrap(traits.hash(key), min_int, hash_max); while (true) { const entry = &(self.entries[hashed_key] orelse continue); @@ -199,8 +202,8 @@ pub fn Table(comptime Key: type, comptime Value: type, comptime traits: TableTra debug.assert(self.entries.len > self.count); { - const hash_max = math.min(math.max_int(@typeInfo(usize).Int), self.entries.len); - var hashed_key = math.wrap(traits.hash(key), math.min_int(@typeInfo(usize).Int), hash_max); + const hash_max = @min(max_int, self.entries.len); + var hashed_key = math.wrap(traits.hash(key), min_int, hash_max); while (true) { const entry = &(self.entries[hashed_key] orelse { @@ -273,8 +276,8 @@ pub fn Table(comptime Key: type, comptime Value: type, comptime traits: TableTra return null; } - const hash_max = math.min(math.max_int(@typeInfo(usize).Int), self.entries.len); - var hashed_key = math.wrap(traits.hash(key), math.min_int(@typeInfo(usize).Int), hash_max); + const hash_max = @min(max_int, self.entries.len); + var hashed_key = math.wrap(traits.hash(key), min_int, hash_max); var iterations = @as(usize, 0); while (iterations < self.count) : (iterations += 1) { @@ -303,7 +306,7 @@ pub fn Table(comptime Key: type, comptime Value: type, comptime traits: TableTra return; } - const min_count = math.max(1, self.count); + const min_count = @max(1, self.count); const table_size = min_count * 2; const allocation = @as([*]?Entry, @ptrCast(@alignCast(try self.allocator.reallocate(null, @sizeOf(?Entry) * table_size))))[0 .. table_size]; diff --git a/source/coral/math.zig b/source/coral/math.zig index b0cfb32..d5883cf 100644 --- a/source/coral/math.zig +++ b/source/coral/math.zig @@ -1,7 +1,24 @@ const std = @import("std"); -pub fn max(a: anytype, b: anytype) @TypeOf(a, b) { - return @max(a, b); +pub fn Int(comptime int: std.builtin.Type.Int) type { + return @Type(.{.Int = int}); +} + +pub fn clamp(value: anytype, lower: anytype, upper: anytype) @TypeOf(value, lower, upper) { + return @max(lower, @min(upper, value)); +} + +pub fn clamped_cast(comptime dest_int: std.builtin.Type.Int, value: anytype) Int(dest_int) { + const Value = @TypeOf(value); + + return switch (@typeInfo(Value)) { + .Int => |int| switch (int.signedness) { + .signed => @intCast(clamp(value, min_int(dest_int), max_int(dest_int))), + .unsigned => @intCast(@min(value, max_int(dest_int))), + }, + + else => @compileError("`" ++ @typeName(Value) ++ "` cannot be cast to an int"), + }; } pub fn max_int(comptime int: std.builtin.Type.Int) comptime_int { @@ -12,10 +29,6 @@ pub fn max_int(comptime int: std.builtin.Type.Int) comptime_int { return (1 << (bit_count - @intFromBool(int.signedness == .signed))) - 1; } -pub fn min(a: anytype, b: anytype) @TypeOf(a, b) { - return @min(a, b); -} - pub fn min_int(comptime int: std.builtin.Type.Int) comptime_int { if (int.signedness == .unsigned) return 0; diff --git a/source/coral/utf8.zig b/source/coral/utf8.zig index 48b6199..62fccbc 100644 --- a/source/coral/utf8.zig +++ b/source/coral/utf8.zig @@ -126,7 +126,7 @@ pub const DecimalFormat = struct { switch (@typeInfo(ValueType)) { .Int => |int| { const radix = 10; - var buffer = [_]u8{0} ** (1 + math.max(int.bits, 1)); + var buffer = [_]u8{0} ** (1 + @max(int.bits, 1)); var buffer_start = buffer.len - 1; { diff --git a/source/ona/app.zig b/source/ona/app.zig index bd4117e..6450445 100644 --- a/source/ona/app.zig +++ b/source/ona/app.zig @@ -23,8 +23,9 @@ pub const Manifest = struct { defer env.discard(ref); if (env.unbox(ref).expect_number()) |number| { - // TODO: Add safety-checks to int cast. - break: get @intFromFloat(number); + if (number > 0 and number < coral.math.max_int(@typeInfo(@TypeOf(self.width)).Int)) { + break: get @intFromFloat(number); + } } break: get self.width; @@ -36,8 +37,9 @@ pub const Manifest = struct { defer env.discard(ref); if (env.unbox(ref).expect_number()) |number| { - // TODO: Add safety-checks to int cast. - break: get @intFromFloat(number); + if (number > 0 and number < coral.math.max_int(@typeInfo(@TypeOf(self.height)).Int)) { + break: get @intFromFloat(number); + } } break: get self.height; @@ -49,7 +51,6 @@ pub const Manifest = struct { defer env.discard(ref); if (env.unbox(ref).expect_number()) |number| { - // TODO: Add safety-checks to int cast. break: get @floatCast(number); } @@ -62,7 +63,7 @@ pub const Manifest = struct { defer env.discard(title_ref); const title_string = env.unbox(title_ref).expect_string() orelse ""; - const limited_title_len = coral.math.min(title_string.len, self.title.len); + const limited_title_len = @min(title_string.len, self.title.len); coral.io.copy(&self.title, title_string[0 .. limited_title_len]); coral.io.zero(self.title[limited_title_len .. self.title.len]); @@ -75,13 +76,28 @@ pub const Manifest = struct { }; pub fn log_info(message: []const coral.io.Byte) void { - ext.SDL_LogInfo(ext.SDL_LOG_CATEGORY_APPLICATION, "%.*s", @as(c_int, @intCast(message.len)), message.ptr); + ext.SDL_LogInfo( + ext.SDL_LOG_CATEGORY_APPLICATION, + "%.*s", + coral.math.clamped_cast(@typeInfo(c_int).Int, message.len), + message.ptr, + ); } pub fn log_warn(message: []const coral.io.Byte) void { - ext.SDL_LogWarn(ext.SDL_LOG_CATEGORY_APPLICATION, "%.*s", @as(c_int, @intCast(message.len)), message.ptr); + ext.SDL_LogWarn( + ext.SDL_LOG_CATEGORY_APPLICATION, + "%.*s", + coral.math.clamped_cast(@typeInfo(c_int).Int, message.len), + message.ptr, + ); } pub fn log_fail(message: []const coral.io.Byte) void { - ext.SDL_LogError(ext.SDL_LOG_CATEGORY_APPLICATION, "%.*s", @as(c_int, @intCast(message.len)), message.ptr); + ext.SDL_LogError( + ext.SDL_LOG_CATEGORY_APPLICATION, + "%.*s", + coral.math.clamped_cast(@typeInfo(c_int).Int, message.len), + message.ptr, + ); } diff --git a/source/ona/kym.zig b/source/ona/kym.zig index 178311b..f21b08f 100644 --- a/source/ona/kym.zig +++ b/source/ona/kym.zig @@ -238,7 +238,7 @@ pub const RuntimeEnv = struct { return self.raise(error.IllegalState, "out of bounds local get"); } - return self.local_refs.values[local]; + return self.acquire(self.local_refs.values[local]); } pub fn pop_local(self: *RuntimeEnv) RuntimeError!?*RuntimeRef { diff --git a/source/ona/kym/Ast.zig b/source/ona/kym/Ast.zig index c5781f3..cb2855c 100755 --- a/source/ona/kym/Ast.zig +++ b/source/ona/kym/Ast.zig @@ -192,7 +192,7 @@ pub fn parse(self: *Self, tokenizer: *tokens.Tokenizer) ParseError!void { has_returned = true; }, - .local => |identifier| { + .identifier => |identifier| { tokenizer.step(); switch (tokenizer.token orelse return self.raise(no_effect_message)) { @@ -221,7 +221,7 @@ pub fn parse(self: *Self, tokenizer: *tokens.Tokenizer) ParseError!void { } }, - .global => |identifier| { + .special_identifier => |identifier| { tokenizer.step(); switch (tokenizer.token orelse return self.raise(no_effect_message)) { @@ -333,7 +333,7 @@ fn parse_factor(self: *Self, tokenizer: *tokens.Tokenizer) ParseError!Expression return Expression{.string_literal = value}; }, - .global => |identifier| { + .special_identifier => |identifier| { tokenizer.skip(.newline); var expression_list = Expression.List.make(allocator); @@ -375,7 +375,7 @@ fn parse_factor(self: *Self, tokenizer: *tokens.Tokenizer) ParseError!Expression } }, - .local => |identifier| { + .identifier => |identifier| { tokenizer.step(); return Expression{.get_local = identifier}; @@ -394,7 +394,7 @@ fn parse_factor(self: *Self, tokenizer: *tokens.Tokenizer) ParseError!Expression return Expression{.table_literal = table_fields}; }, - .local => |identifier| { + .identifier => |identifier| { tokenizer.skip(.newline); if (!tokenizer.is_token(.symbol_equals)) { diff --git a/source/ona/kym/Table.zig b/source/ona/kym/Table.zig index 09ae165..11506ed 100644 --- a/source/ona/kym/Table.zig +++ b/source/ona/kym/Table.zig @@ -23,7 +23,7 @@ pub fn new(env: *kym.RuntimeEnv) kym.RuntimeError!?*kym.RuntimeRef { return try env.new_dynamic(coral.io.bytes_of(&self), &typeinfo); } -pub const typeinfo = kym.Typeinfo{ +const typeinfo = kym.Typeinfo{ .name = "table", .clean = typeinfo_clean, .get = typeinfo_get, diff --git a/source/ona/kym/tokens.zig b/source/ona/kym/tokens.zig index 43f4d2c..49e8d0d 100755 --- a/source/ona/kym/tokens.zig +++ b/source/ona/kym/tokens.zig @@ -1,11 +1,11 @@ const coral = @import("coral"); pub const Token = union(enum) { - unknown: u8, + unknown: coral.io.Byte, newline, - global: []const u8, - local: []const u8, + special_identifier: []const coral.io.Byte, + identifier: []const coral.io.Byte, symbol_plus, symbol_minus, @@ -29,8 +29,8 @@ pub const Token = union(enum) { symbol_equals, symbol_double_equals, - number: []const u8, - string: []const u8, + number: []const coral.io.Byte, + string: []const coral.io.Byte, keyword_nil, keyword_false, @@ -39,13 +39,13 @@ pub const Token = union(enum) { keyword_self, keyword_const, - pub fn text(self: Token) []const u8 { + pub fn text(self: Token) []const coral.io.Byte { return switch (self) { - .unknown => |unknown| @as([*]const u8, @ptrCast(&unknown))[0 .. 1], + .unknown => |unknown| @as([*]const coral.io.Byte, @ptrCast(&unknown))[0 .. 1], .newline => "newline", - .global => |identifier| identifier, - .local => |identifier| identifier, + .special_identifier => |identifier| identifier, + .identifier => |identifier| identifier, .symbol_plus => "+", .symbol_minus => "-", @@ -83,7 +83,7 @@ pub const Token = union(enum) { }; pub const Tokenizer = struct { - source: []const u8, + source: []const coral.io.Byte, lines_stepped: usize = 1, token: ?Token = null, @@ -217,7 +217,7 @@ pub const Tokenizer = struct { else => {}, } - self.token = .{.local = identifier}; + self.token = .{.identifier = identifier}; return; }, @@ -236,7 +236,7 @@ pub const Tokenizer = struct { else => break, }; - self.token = .{.global = self.source[begin .. cursor]}; + self.token = .{.special_identifier = self.source[begin .. cursor]}; return; }, @@ -253,7 +253,7 @@ pub const Tokenizer = struct { else => cursor += 1, }; - self.token = .{.global = self.source[begin .. cursor]}; + self.token = .{.special_identifier = self.source[begin .. cursor]}; cursor += 1; return; diff --git a/source/ona/ona.zig b/source/ona/ona.zig index a0573d1..a7ef131 100644 --- a/source/ona/ona.zig +++ b/source/ona/ona.zig @@ -67,7 +67,7 @@ pub fn run_app(file_access: file.Access) void { .caller = kym.Caller.from(kym_log_fail), }, }) catch { - return app.log_fail("failed to initialize script runtime"); + return app.log_fail("failed to bind syscalls to script runtime"); }; var manifest = app.Manifest{}; diff --git a/source/runner.zig b/source/runner.zig index d38ba14..61dd7b5 100644 --- a/source/runner.zig +++ b/source/runner.zig @@ -1,5 +1,5 @@ const ona = @import("ona"); -pub fn main() anyerror!void { +pub fn main() void { ona.run_app(.{.sandboxed_path = &ona.file.Path.cwd}); }