From 3b6ae6807cc78e1a487f03ddbfa23355be32993a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 21 Mar 2024 20:45:57 -0700 Subject: [PATCH] terminal: add more integrity assertions --- src/terminal/Screen.zig | 15 ++++++++ src/terminal/Terminal.zig | 15 ++++++++ src/terminal/page.zig | 76 +++++++++++++++------------------------ 3 files changed, 59 insertions(+), 47 deletions(-) diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 0434a66aa..adac2951e 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -709,6 +709,8 @@ pub const Scroll = union(enum) { /// Scroll the viewport of the terminal grid. pub fn scroll(self: *Screen, behavior: Scroll) void { + defer self.assertIntegrity(); + // No matter what, scrolling marks our image state as dirty since // it could move placements. If there are no placements or no images // this is still a very cheap operation. @@ -726,6 +728,8 @@ pub fn scroll(self: *Screen, behavior: Scroll) void { /// See PageList.scrollClear. In addition to that, we reset the cursor /// to be on top. pub fn scrollClear(self: *Screen) !void { + defer self.assertIntegrity(); + try self.pages.scrollClear(); self.cursorReload(); @@ -748,6 +752,8 @@ pub fn eraseRows( tl: point.Point, bl: ?point.Point, ) void { + defer self.assertIntegrity(); + // Erase the rows self.pages.eraseRows(tl, bl); @@ -769,6 +775,8 @@ pub fn clearRows( bl: ?point.Point, protected: bool, ) void { + defer self.assertIntegrity(); + var it = self.pages.pageIterator(.right_down, tl, bl); while (it.next()) |chunk| { for (chunk.rows()) |*row| { @@ -842,6 +850,8 @@ pub fn clearCells( } @memset(cells, self.blankCell()); + page.assertIntegrity(); + self.assertIntegrity(); } /// Clear cells but only if they are not protected. @@ -856,6 +866,9 @@ pub fn clearUnprotectedCells( const cell_multi: [*]Cell = @ptrCast(cell); self.clearCells(page, row, cell_multi[0..1]); } + + page.assertIntegrity(); + self.assertIntegrity(); } /// Clears the prompt lines if the cursor is currently at a prompt. This @@ -977,6 +990,7 @@ fn resizeInternal( // If our cursor was updated, we do a full reload so all our cursor // state is correct. self.cursorReload(); + self.assertIntegrity(); } /// Set a style attribute for the current cursor. @@ -1192,6 +1206,7 @@ pub fn manualStyleUpdate(self: *Screen) !void { self.cursor.style_id = md.id; self.cursor.style_ref = &md.ref; if (cursor_reload) self.cursorReload(); + self.assertIntegrity(); } /// Append a grapheme to the given cell within the current cursor row. diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index c89759be3..c8cad3780 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -223,6 +223,10 @@ pub fn print(self: *Terminal, c: u21) !void { // If we're not on the main display, do nothing for now if (self.status_display != .main) return; + // After doing any printing, wrapping, scrolling, etc. we want to ensure + // that our screen remains in a consistent state. + defer self.screen.assertIntegrity(); + // Our right margin depends where our cursor is now. const right_limit = if (self.screen.cursor.x > self.scrolling_region.right) self.cols @@ -470,6 +474,8 @@ fn printCell( unmapped_c: u21, wide: Cell.Wide, ) void { + defer self.screen.assertIntegrity(); + // TODO: spacers should use a bgcolor only cell const c: u21 = c: { @@ -627,6 +633,9 @@ fn printWrap(self: *Terminal) !void { // New line must inherit semantic prompt of the old line self.screen.cursor.page_row.semantic_prompt = old_prompt; self.screen.cursor.page_row.wrap_continuation = true; + + // Assure that our screen is consistent + self.screen.assertIntegrity(); } /// Set the charset into the given slot. @@ -876,6 +885,9 @@ pub fn restoreCursor(self: *Terminal) !void { @min(saved.x, self.cols - 1), @min(saved.y, self.rows - 1), ); + + // Ensure our screen is consistent + self.screen.assertIntegrity(); } /// Set the character protection mode for the terminal. @@ -1315,6 +1327,9 @@ pub fn insertLines(self: *Terminal, count: usize) void { const dst_row = dst.*; dst.* = src.*; src.* = dst_row; + + // Ensure what we did didn't corrupt the page + p.page.data.assertIntegrity(); continue; } diff --git a/src/terminal/page.zig b/src/terminal/page.zig index 2b36ed92e..3438f7ec3 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -176,7 +176,6 @@ pub const Page = struct { pub const IntegrityError = error{ UnmarkedGraphemeRow, - MarkedGraphemeRow, MissingGraphemeData, InvalidGraphemeCount, MissingStyle, @@ -267,16 +266,6 @@ pub const Page = struct { ); return IntegrityError.UnmarkedGraphemeRow; } - } else { - // If no cells in a row have grapheme data, the row must - // not be marked as having grapheme data. - if (row.grapheme) { - log.warn( - "page integrity violation y={} row marked but no grapheme data", - .{y}, - ); - return IntegrityError.MarkedGraphemeRow; - } } } @@ -406,6 +395,9 @@ pub const Page = struct { dst_row, src_row, ); + + // We should remain consistent + self.assertIntegrity(); } /// Clone a single row from another page into this page. @@ -454,6 +446,9 @@ pub const Page = struct { dst_cell.style_id = md.id; } } + + // The final page should remain consistent + self.assertIntegrity(); } /// Get a single row. y must be valid. @@ -501,6 +496,8 @@ pub const Page = struct { dst_left: usize, len: usize, ) void { + defer self.assertIntegrity(); + const src_cells = src_row.cells.ptr(self.memory)[src_left .. src_left + len]; const dst_cells = dst_row.cells.ptr(self.memory)[dst_left .. dst_left + len]; @@ -527,6 +524,9 @@ pub const Page = struct { dst.content_tag = .codepoint; self.moveGrapheme(src, dst); } + + // The destination row must be marked + dst_row.grapheme = true; } // Clear our source row now that the copy is complete @@ -544,6 +544,8 @@ pub const Page = struct { left: usize, end: usize, ) void { + defer self.assertIntegrity(); + const cells = row.cells.ptr(self.memory)[left..end]; if (row.grapheme) { for (cells) |*cell| { @@ -572,6 +574,8 @@ pub const Page = struct { /// Append a codepoint to the given cell as a grapheme. pub fn appendGrapheme(self: *Page, row: *Row, cell: *Cell, cp: u21) Allocator.Error!void { + defer self.assertIntegrity(); + if (comptime std.debug.runtime_safety) assert(cell.hasText()); const cell_offset = getOffset(Cell, self.memory, cell); @@ -640,7 +644,7 @@ pub const Page = struct { /// Move the graphemes from one cell to another. This can't fail /// because we avoid any allocations since we're just moving data. - pub fn moveGrapheme(self: *const Page, src: *Cell, dst: *Cell) void { + pub fn moveGrapheme(self: *Page, src: *Cell, dst: *Cell) void { if (comptime std.debug.runtime_safety) { assert(src.hasGrapheme()); assert(!dst.hasGrapheme()); @@ -654,10 +658,12 @@ pub const Page = struct { map.removeByPtr(entry.key_ptr); map.putAssumeCapacity(dst_offset, value); src.content_tag = .codepoint; + dst.content_tag = .codepoint_grapheme; } /// Clear the graphemes for a given cell. pub fn clearGrapheme(self: *Page, row: *Row, cell: *Cell) void { + defer self.assertIntegrity(); if (comptime std.debug.runtime_safety) assert(cell.hasGrapheme()); // Get our entry in the map, which must exist @@ -692,6 +698,8 @@ pub const Page = struct { // the places we call this is from insertBlanks where the cells have // already swapped cell data but not grapheme data. + defer self.assertIntegrity(); + // Get our entry in the map, which must exist const src_offset = getOffset(Cell, self.memory, src); var map = self.grapheme_map.map(self.memory); @@ -1575,7 +1583,7 @@ test "Page moveCells graphemes" { defer page.deinit(); // Write - for (0..page.capacity.cols) |x| { + for (0..page.size.cols) |x| { const rac = page.getRowAndCell(x, 0); rac.cell.* = .{ .content_tag = .codepoint, @@ -1587,11 +1595,11 @@ test "Page moveCells graphemes" { const src = page.getRow(0); const dst = page.getRow(1); - page.moveCells(src, 0, dst, 0, page.capacity.cols); + page.moveCells(src, 0, dst, 0, page.size.cols); try testing.expectEqual(original_count, page.graphemeCount()); // New rows should have text - for (0..page.capacity.cols) |x| { + for (0..page.size.cols) |x| { const rac = page.getRowAndCell(x, 1); try testing.expectEqual( @as(u21, @intCast(x + 1)), @@ -1605,7 +1613,7 @@ test "Page moveCells graphemes" { } // Old row should be blank - for (0..page.capacity.cols) |x| { + for (0..page.size.cols) |x| { const rac = page.getRowAndCell(x, 0); try testing.expectEqual( @as(u21, 0), @@ -1623,7 +1631,7 @@ test "Page verifyIntegrity graphemes good" { defer page.deinit(); // Write - for (0..page.capacity.cols) |x| { + for (0..page.size.cols) |x| { const rac = page.getRowAndCell(x, 0); rac.cell.* = .{ .content_tag = .codepoint, @@ -1644,7 +1652,7 @@ test "Page verifyIntegrity grapheme row not marked" { defer page.deinit(); // Write - for (0..page.capacity.cols) |x| { + for (0..page.size.cols) |x| { const rac = page.getRowAndCell(x, 0); rac.cell.* = .{ .content_tag = .codepoint, @@ -1662,32 +1670,6 @@ test "Page verifyIntegrity grapheme row not marked" { ); } -test "Page verifyIntegrity text row marked as grapheme" { - var page = try Page.init(.{ - .cols = 10, - .rows = 10, - .styles = 8, - }); - defer page.deinit(); - - // Write - for (0..page.capacity.cols) |x| { - const rac = page.getRowAndCell(x, 0); - rac.cell.* = .{ - .content_tag = .codepoint, - .content = .{ .codepoint = @intCast(x + 1) }, - }; - } - - // Make invalid by unmarking the row - page.getRow(0).grapheme = true; - - try testing.expectError( - Page.IntegrityError.MarkedGraphemeRow, - page.verifyIntegrity(testing.allocator), - ); -} - test "Page verifyIntegrity styles good" { var page = try Page.init(.{ .cols = 10, @@ -1702,7 +1684,7 @@ test "Page verifyIntegrity styles good" { } }); // Write - for (0..page.capacity.cols) |x| { + for (0..page.size.cols) |x| { const rac = page.getRowAndCell(x, 0); rac.row.styled = true; rac.cell.* = .{ @@ -1730,7 +1712,7 @@ test "Page verifyIntegrity styles ref count mismatch" { } }); // Write - for (0..page.capacity.cols) |x| { + for (0..page.size.cols) |x| { const rac = page.getRowAndCell(x, 0); rac.row.styled = true; rac.cell.* = .{ @@ -1769,7 +1751,7 @@ test "Page verifyIntegrity styles extra" { md2.ref += 1; // Write - for (0..page.capacity.cols) |x| { + for (0..page.size.cols) |x| { const rac = page.getRowAndCell(x, 0); rac.row.styled = true; rac.cell.* = .{