Skip to content

Commit

Permalink
terminal: make error sets more explicit, capture explicit errors
Browse files Browse the repository at this point in the history
  • Loading branch information
mitchellh committed Aug 31, 2024
1 parent 3807ee3 commit 9d08ed3
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 23 deletions.
24 changes: 16 additions & 8 deletions src/terminal/PageList.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1758,6 +1758,8 @@ pub const AdjustCapacity = struct {
string_bytes: ?usize = null,
};

pub const AdjustCapacityError = Allocator.Error || Page.CloneFromError;

/// Adjust the capcaity of the given page in the list. This should
/// be used in cases where OutOfMemory is returned by some operation
/// i.e to increase style counts, grapheme counts, etc.
Expand All @@ -1778,25 +1780,31 @@ pub fn adjustCapacity(
self: *PageList,
page: *List.Node,
adjustment: AdjustCapacity,
) !*List.Node {
) AdjustCapacityError!*List.Node {
// We always start with the base capacity of the existing page. This
// ensures we never shrink from what we need.
var cap = page.data.capacity;

// All ceilPowerOfTwo is unreachable because we're always same or less
// bit width so maxInt is always possible.
if (adjustment.styles) |v| {
const aligned = try std.math.ceilPowerOfTwo(usize, v);
comptime assert(@bitSizeOf(@TypeOf(v)) <= @bitSizeOf(usize));
const aligned = std.math.ceilPowerOfTwo(usize, v) catch unreachable;
cap.styles = @max(cap.styles, aligned);
}
if (adjustment.grapheme_bytes) |v| {
const aligned = try std.math.ceilPowerOfTwo(usize, v);
comptime assert(@bitSizeOf(@TypeOf(v)) <= @bitSizeOf(usize));
const aligned = std.math.ceilPowerOfTwo(usize, v) catch unreachable;
cap.grapheme_bytes = @max(cap.grapheme_bytes, aligned);
}
if (adjustment.hyperlink_bytes) |v| {
const aligned = try std.math.ceilPowerOfTwo(usize, v);
comptime assert(@bitSizeOf(@TypeOf(v)) <= @bitSizeOf(usize));
const aligned = std.math.ceilPowerOfTwo(usize, v) catch unreachable;
cap.hyperlink_bytes = @max(cap.hyperlink_bytes, aligned);
}
if (adjustment.string_bytes) |v| {
const aligned = try std.math.ceilPowerOfTwo(usize, v);
comptime assert(@bitSizeOf(@TypeOf(v)) <= @bitSizeOf(usize));
const aligned = std.math.ceilPowerOfTwo(usize, v) catch unreachable;
cap.string_bytes = @max(cap.string_bytes, aligned);
}

Expand Down Expand Up @@ -1830,7 +1838,7 @@ pub fn adjustCapacity(
fn createPage(
self: *PageList,
cap: Capacity,
) !*List.Node {
) Allocator.Error!*List.Node {
// log.debug("create page cap={}", .{cap});
return try createPageExt(&self.pool, cap, &self.page_size);
}
Expand All @@ -1839,7 +1847,7 @@ fn createPageExt(
pool: *MemoryPool,
cap: Capacity,
total_size: ?*usize,
) !*List.Node {
) Allocator.Error!*List.Node {
var page = try pool.nodes.create();
errdefer pool.nodes.destroy(page);

Expand Down Expand Up @@ -2292,7 +2300,7 @@ pub fn pin(self: *const PageList, pt: point.Point) ?Pin {
/// automatically updated as the pagelist is modified. If the point the
/// pin points to is removed completely, the tracked pin will be updated
/// to the top-left of the screen.
pub fn trackPin(self: *PageList, p: Pin) !*Pin {
pub fn trackPin(self: *PageList, p: Pin) Allocator.Error!*Pin {
if (comptime std.debug.runtime_safety) assert(self.pinIsValid(p));

// Create our tracked pin
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/Screen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ pub fn adjustCapacity(
self: *Screen,
page: *PageList.List.Node,
adjustment: PageList.AdjustCapacity,
) !*PageList.List.Node {
) PageList.AdjustCapacityError!*PageList.List.Node {
// If the page being modified isn't our cursor page then
// this is a quick operation because we have no additional
// accounting.
Expand Down
63 changes: 51 additions & 12 deletions src/terminal/Terminal.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1444,6 +1444,7 @@ pub fn insertLines(self: *Terminal, count: usize) void {
const start_y = self.screen.cursor.y;
defer {
self.screen.cursorAbsolute(self.scrolling_region.left, start_y);

// Always unset pending wrap
self.screen.cursor.pending_wrap = false;
}
Expand All @@ -1464,8 +1465,15 @@ pub fn insertLines(self: *Terminal, count: usize) void {
var cur_p = self.screen.pages.trackPin(
self.screen.cursor.page_pin.down(rem - 1).?,
) catch |err| {
std.log.err("TODO: insertLines handle trackPin error err={}", .{err});
@panic("TODO: insertLines handle trackPin error");
comptime assert(@TypeOf(err) == error{OutOfMemory});

// This error scenario means that our GPA is OOM. This is not a
// situation we can gracefully handle. We can't just ignore insertLines
// because it'll result in a corrupted screen. Ideally in the future
// we flag the state as broken and show an error message to the user.
// For now, we panic.
log.err("insertLines trackPin error err={}", .{err});
@panic("insertLines trackPin OOM");
};
defer self.screen.pages.untrackPin(cur_p);

Expand Down Expand Up @@ -1525,7 +1533,7 @@ pub fn insertLines(self: *Terminal, count: usize) void {

// Increase style memory
error.StyleSetOutOfMemory,
=> .{.styles = cap.styles * 2 },
=> .{ .styles = cap.styles * 2 },

// Increase string memory
error.StringAllocOutOfMemory,
Expand All @@ -1541,10 +1549,27 @@ pub fn insertLines(self: *Terminal, count: usize) void {
error.GraphemeAllocOutOfMemory,
=> .{ .grapheme_bytes = cap.grapheme_bytes * 2 },
},
) catch |e| {
std.log.err("TODO: insertLines handle adjustCapacity error err={}", .{e});
@panic("TODO: insertLines handle adjustCapacity error");
) catch |e| switch (e) {
// This shouldn't be possible because above we're only
// adjusting capacity _upwards_. So it should have all
// the existing capacity it had to fit the adjusted
// data. Panic since we don't expect this.
error.StyleSetOutOfMemory,
error.StyleSetNeedsRehash,
error.StringAllocOutOfMemory,
error.HyperlinkSetOutOfMemory,
error.HyperlinkSetNeedsRehash,
error.HyperlinkMapOutOfMemory,
error.GraphemeMapOutOfMemory,
error.GraphemeAllocOutOfMemory,
=> @panic("adjustCapacity resulted in capacity errors"),

// The system allocator is OOM. We can't currently do
// anything graceful here. We panic.
error.OutOfMemory,
=> @panic("adjustCapacity system allocator OOM"),
};

// Continue the loop to try handling this row again.
continue;
};
Expand Down Expand Up @@ -1643,8 +1668,10 @@ pub fn deleteLines(self: *Terminal, count: usize) void {
var cur_p = self.screen.pages.trackPin(
self.screen.cursor.page_pin.*,
) catch |err| {
std.log.err("TODO: deleteLines handle trackPin error err={}", .{err});
@panic("TODO: deleteLines handle trackPin error");
// See insertLines
comptime assert(@TypeOf(err) == error{OutOfMemory});
log.err("deleteLines trackPin error err={}", .{err});
@panic("deleteLines trackPin OOM");
};
defer self.screen.pages.untrackPin(cur_p);

Expand Down Expand Up @@ -1704,7 +1731,7 @@ pub fn deleteLines(self: *Terminal, count: usize) void {

// Increase style memory
error.StyleSetOutOfMemory,
=> .{.styles = cap.styles * 2 },
=> .{ .styles = cap.styles * 2 },

// Increase string memory
error.StringAllocOutOfMemory,
Expand All @@ -1720,10 +1747,22 @@ pub fn deleteLines(self: *Terminal, count: usize) void {
error.GraphemeAllocOutOfMemory,
=> .{ .grapheme_bytes = cap.grapheme_bytes * 2 },
},
) catch |e| {
std.log.err("TODO: deleteLines handle adjustCapacity error err={}", .{e});
@panic("TODO: deleteLines handle adjustCapacity error");
) catch |e| switch (e) {
// See insertLines which has the same error capture.
error.StyleSetOutOfMemory,
error.StyleSetNeedsRehash,
error.StringAllocOutOfMemory,
error.HyperlinkSetOutOfMemory,
error.HyperlinkSetNeedsRehash,
error.HyperlinkMapOutOfMemory,
error.GraphemeMapOutOfMemory,
error.GraphemeAllocOutOfMemory,
=> @panic("adjustCapacity resulted in capacity errors"),

error.OutOfMemory,
=> @panic("adjustCapacity system allocator OOM"),
};

// Continue the loop to try handling this row again.
continue;
};
Expand Down
7 changes: 5 additions & 2 deletions src/terminal/page.zig
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ pub const Page = struct {
const cells = dst_row.cells.ptr(self.memory)[x_start..x_end];

// If our destination has styles or graphemes then we need to
// clear some state.
// clear some state. This will free up the managed memory as well.
if (dst_row.managedMemory()) self.clearCells(dst_row, x_start, x_end);

// Copy all the row metadata but keep our cells offset
Expand Down Expand Up @@ -771,6 +771,8 @@ pub const Page = struct {
// Copy the grapheme codepoints
const cps = other.lookupGrapheme(src_cell).?;

// Safe to use setGraphemes because we cleared all
// managed memory for our destination cell range.
try self.setGraphemes(dst_row, dst_cell, cps);
}
if (src_cell.hyperlink) hyperlink: {
Expand Down Expand Up @@ -801,10 +803,11 @@ pub const Page = struct {
if (self.hyperlink_set.lookupContext(
self.memory,
other_link.*,
.{ .page = @constCast(other) },

// `lookupContext` uses the context for hashing, and
// that doesn't write to the page, so this constCast
// is completely safe.
.{ .page = @constCast(other) },
)) |i| {
self.hyperlink_set.use(self.memory, i);
break :dst_id i;
Expand Down

0 comments on commit 9d08ed3

Please sign in to comment.