From 3f1d6eb301a7fb3d967c7f17c555c8dd761d900c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 17 Oct 2024 22:00:05 -0700 Subject: [PATCH] expand explicit error set usage This continues our work to improve the amount of explicit error sets we use in the codebase. Explicit error sets make it easier to understand possible failure scenarios, allow us to use exhaustive matching, create compiler errors if errors are unexpectedly added or removed, etc. The goal eventually is 100% coverage but we're not even close yet. This just moves us a little closer. --- src/App.zig | 27 ++++++++++++++++----------- src/Surface.zig | 2 +- src/config/Config.zig | 33 ++++++++++++++++++++------------- src/font/Collection.zig | 2 +- src/font/SharedGridSet.zig | 11 ++++++++--- src/font/library.zig | 8 ++++++-- 6 files changed, 52 insertions(+), 31 deletions(-) diff --git a/src/App.zig b/src/App.zig index 0d09fed58..b1ea2eb7b 100644 --- a/src/App.zig +++ b/src/App.zig @@ -66,6 +66,8 @@ font_grid_set: font.SharedGridSet, last_notification_time: ?std.time.Instant = null, last_notification_digest: u64 = 0, +pub const CreateError = Allocator.Error || font.SharedGridSet.InitError; + /// Initialize the main app instance. This creates the main window, sets /// up the renderer state, compiles the shaders, etc. This is the primary /// "startup" logic. @@ -74,7 +76,7 @@ last_notification_digest: u64 = 0, /// `focusEvent` to set the initial focus state of the app. pub fn create( alloc: Allocator, -) !*App { +) CreateError!*App { var app = try alloc.create(App); errdefer alloc.destroy(app); @@ -150,7 +152,10 @@ pub fn updateConfig(self: *App, config: *const Config) !void { /// Add an initialized surface. This is really only for the runtime /// implementations to call and should NOT be called by general app users. /// The surface must be from the pool. -pub fn addSurface(self: *App, rt_surface: *apprt.Surface) !void { +pub fn addSurface( + self: *App, + rt_surface: *apprt.Surface, +) Allocator.Error!void { try self.surfaces.append(self.alloc, rt_surface); // Since we have non-zero surfaces, we can cancel the quit timer. @@ -225,11 +230,11 @@ fn drainMailbox(self: *App, rt_app: *apprt.App) !void { .reload_config => try self.reloadConfig(rt_app), .open_config => try self.performAction(rt_app, .open_config), .new_window => |msg| try self.newWindow(rt_app, msg), - .close => |surface| try self.closeSurface(surface), - .quit => try self.setQuit(), + .close => |surface| self.closeSurface(surface), + .quit => self.setQuit(), .surface_message => |msg| try self.surfaceMessage(msg.surface, msg.message), - .redraw_surface => |surface| try self.redrawSurface(rt_app, surface), - .redraw_inspector => |surface| try self.redrawInspector(rt_app, surface), + .redraw_surface => |surface| self.redrawSurface(rt_app, surface), + .redraw_inspector => |surface| self.redrawInspector(rt_app, surface), } } } @@ -242,7 +247,7 @@ pub fn reloadConfig(self: *App, rt_app: *apprt.App) !void { } } -pub fn closeSurface(self: *App, surface: *Surface) !void { +pub fn closeSurface(self: *App, surface: *Surface) void { if (!self.hasSurface(surface)) return; surface.close(); } @@ -252,12 +257,12 @@ pub fn focusSurface(self: *App, surface: *Surface) void { self.focused_surface = surface; } -fn redrawSurface(self: *App, rt_app: *apprt.App, surface: *apprt.Surface) !void { +fn redrawSurface(self: *App, rt_app: *apprt.App, surface: *apprt.Surface) void { if (!self.hasSurface(&surface.core_surface)) return; rt_app.redrawSurface(surface); } -fn redrawInspector(self: *App, rt_app: *apprt.App, surface: *apprt.Surface) !void { +fn redrawInspector(self: *App, rt_app: *apprt.App, surface: *apprt.Surface) void { if (!self.hasSurface(&surface.core_surface)) return; rt_app.redrawInspector(surface); } @@ -278,7 +283,7 @@ pub fn newWindow(self: *App, rt_app: *apprt.App, msg: Message.NewWindow) !void { } /// Start quitting -pub fn setQuit(self: *App) !void { +pub fn setQuit(self: *App) void { if (self.quit) return; self.quit = true; } @@ -373,7 +378,7 @@ pub fn performAction( switch (action) { .unbind => unreachable, .ignore => {}, - .quit => try self.setQuit(), + .quit => self.setQuit(), .new_window => try self.newWindow(rt_app, .{ .parent = null }), .open_config => try rt_app.performAction(.app, .open_config, {}), .reload_config => try self.reloadConfig(rt_app), diff --git a/src/Surface.zig b/src/Surface.zig index 116b98214..c1b7afd7b 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -4011,7 +4011,7 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !bool .close_surface => self.close(), - .close_window => try self.app.closeSurface(self), + .close_window => self.app.closeSurface(self), .crash => |location| switch (location) { .main => @panic("crash binding action, crashing intentionally"), diff --git a/src/config/Config.zig b/src/config/Config.zig index 765b44db1..a5c337fe6 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2758,7 +2758,10 @@ pub fn shallowClone(self: *const Config, alloc_gpa: Allocator) Config { /// Create a copy of this configuration. This is useful as a starting /// point for modifying a configuration since a config can NOT be /// modified once it is in use by an app or surface. -pub fn clone(self: *const Config, alloc_gpa: Allocator) !Config { +pub fn clone( + self: *const Config, + alloc_gpa: Allocator, +) Allocator.Error!Config { // Start with an empty config with a new arena we're going // to use for all our copies. var result: Config = .{ @@ -2779,7 +2782,11 @@ pub fn clone(self: *const Config, alloc_gpa: Allocator) !Config { return result; } -fn cloneValue(alloc: Allocator, comptime T: type, src: T) !T { +fn cloneValue( + alloc: Allocator, + comptime T: type, + src: T, +) Allocator.Error!T { // Do known named types first switch (T) { []const u8 => return try alloc.dupe(u8, src), @@ -3129,7 +3136,7 @@ pub const Color = packed struct(u24) { } /// Deep copy of the struct. Required by Config. - pub fn clone(self: Color, _: Allocator) !Color { + pub fn clone(self: Color, _: Allocator) error{}!Color { return self; } @@ -3225,7 +3232,7 @@ pub const Palette = struct { } /// Deep copy of the struct. Required by Config. - pub fn clone(self: Self, _: Allocator) !Self { + pub fn clone(self: Self, _: Allocator) error{}!Self { return self; } @@ -3301,7 +3308,7 @@ pub const RepeatableString = struct { } /// Deep copy of the struct. Required by Config. - pub fn clone(self: *const Self, alloc: Allocator) !Self { + pub fn clone(self: *const Self, alloc: Allocator) Allocator.Error!Self { // Copy the list and all the strings in the list. const list = try self.list.clone(alloc); for (list.items) |*item| { @@ -3445,7 +3452,7 @@ pub const RepeatablePath = struct { } /// Deep copy of the struct. Required by Config. - pub fn clone(self: *const Self, alloc: Allocator) !Self { + pub fn clone(self: *const Self, alloc: Allocator) Allocator.Error!Self { const value = try self.value.clone(alloc); for (value.items) |*item| { switch (item.*) { @@ -3656,7 +3663,7 @@ pub const RepeatableFontVariation = struct { } /// Deep copy of the struct. Required by Config. - pub fn clone(self: *const Self, alloc: Allocator) !Self { + pub fn clone(self: *const Self, alloc: Allocator) Allocator.Error!Self { return .{ .list = try self.list.clone(alloc), }; @@ -3789,7 +3796,7 @@ pub const Keybinds = struct { } /// Deep copy of the struct. Required by Config. - pub fn clone(self: *const Keybinds, alloc: Allocator) !Keybinds { + pub fn clone(self: *const Keybinds, alloc: Allocator) Allocator.Error!Keybinds { return .{ .set = try self.set.clone(alloc) }; } @@ -3944,7 +3951,7 @@ pub const RepeatableCodepointMap = struct { } /// Deep copy of the struct. Required by Config. - pub fn clone(self: *const Self, alloc: Allocator) !Self { + pub fn clone(self: *const Self, alloc: Allocator) Allocator.Error!Self { return .{ .map = try self.map.clone(alloc) }; } @@ -4227,7 +4234,7 @@ pub const FontStyle = union(enum) { } /// Deep copy of the struct. Required by Config. - pub fn clone(self: Self, alloc: Allocator) !Self { + pub fn clone(self: Self, alloc: Allocator) Allocator.Error!Self { return switch (self) { .default, .false => self, .name => |v| .{ .name = try alloc.dupeZ(u8, v) }, @@ -4332,7 +4339,7 @@ pub const RepeatableLink = struct { } /// Deep copy of the struct. Required by Config. - pub fn clone(self: *const Self, alloc: Allocator) !Self { + pub fn clone(self: *const Self, alloc: Allocator) error{}!Self { _ = self; _ = alloc; return .{}; @@ -4539,7 +4546,7 @@ pub const Duration = struct { .{ .name = "ns", .factor = 1 }, }; - pub fn clone(self: *const Duration, _: Allocator) !Duration { + pub fn clone(self: *const Duration, _: Allocator) error{}!Duration { return .{ .duration = self.duration }; } @@ -4661,7 +4668,7 @@ pub const WindowPadding = struct { top_left: u32 = 0, bottom_right: u32 = 0, - pub fn clone(self: Self, _: Allocator) !Self { + pub fn clone(self: Self, _: Allocator) error{}!Self { return self; } diff --git a/src/font/Collection.zig b/src/font/Collection.zig index 8f338be16..476787749 100644 --- a/src/font/Collection.zig +++ b/src/font/Collection.zig @@ -118,7 +118,7 @@ pub fn getFace(self: *Collection, index: Index) !*Face { break :item item; }; - return self.getFaceFromEntry(item); + return try self.getFaceFromEntry(item); } /// Get the face from an entry. diff --git a/src/font/SharedGridSet.zig b/src/font/SharedGridSet.zig index f0dd095d5..c3067fa6d 100644 --- a/src/font/SharedGridSet.zig +++ b/src/font/SharedGridSet.zig @@ -46,8 +46,10 @@ font_discover: ?Discover = null, /// Lock to protect multi-threaded access to the map. lock: std.Thread.Mutex = .{}, +pub const InitError = Library.InitError; + /// Initialize a new SharedGridSet. -pub fn init(alloc: Allocator) !SharedGridSet { +pub fn init(alloc: Allocator) InitError!SharedGridSet { var font_lib = try Library.init(); errdefer font_lib.deinit(); @@ -428,7 +430,10 @@ pub const DerivedConfig = struct { /// Initialize a DerivedConfig. The config should be either a /// config.Config or another DerivedConfig to clone from. - pub fn init(alloc_gpa: Allocator, config: anytype) !DerivedConfig { + pub fn init( + alloc_gpa: Allocator, + config: anytype, + ) Allocator.Error!DerivedConfig { var arena = ArenaAllocator.init(alloc_gpa); errdefer arena.deinit(); const alloc = arena.allocator(); @@ -511,7 +516,7 @@ pub const Key = struct { alloc_gpa: Allocator, config_src: *const DerivedConfig, font_size: DesiredSize, - ) !Key { + ) Allocator.Error!Key { var arena = ArenaAllocator.init(alloc_gpa); errdefer arena.deinit(); const alloc = arena.allocator(); diff --git a/src/font/library.zig b/src/font/library.zig index 57e11e64a..b00bbfce0 100644 --- a/src/font/library.zig +++ b/src/font/library.zig @@ -24,7 +24,9 @@ pub const Library = switch (options.backend) { pub const FreetypeLibrary = struct { lib: freetype.Library, - pub fn init() freetype.Error!Library { + pub const InitError = freetype.Error; + + pub fn init() InitError!Library { return Library{ .lib = try freetype.Library.init() }; } @@ -34,7 +36,9 @@ pub const FreetypeLibrary = struct { }; pub const NoopLibrary = struct { - pub fn init() !Library { + pub const InitError = error{}; + + pub fn init() InitError!Library { return Library{}; }