From 8e9cd4bd9e4db3378a9cfbd8ef06e6488180b3de Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 5 Nov 2026 15:35:33 +0100 Subject: [PATCH 1/2] Decommit memory on cls --- .github/actions/spelling/expect/expect.txt | 12 +- src/buffer/out/Row.cpp | 6 + src/buffer/out/Row.hpp | 1 + src/buffer/out/textBuffer.cpp | 286 +++++++++++------- src/buffer/out/textBuffer.hpp | 9 +- .../TerminalBufferTests.cpp | 66 ++++ src/common.build.pre.props | 8 +- src/inc/til/rle.h | 15 +- 8 files changed, 275 insertions(+), 128 deletions(-) diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt index 853fdec7810..59567caebb0 100644 --- a/.github/actions/spelling/expect/expect.txt +++ b/.github/actions/spelling/expect/expect.txt @@ -12,7 +12,6 @@ acp actctx ACTCTXW ADDALIAS -ADDREF ADDSTRING ADDTOOL adml @@ -169,14 +168,11 @@ chshdng CHT CInput CLASSSTRING -CLAUDE cleartype CLICKACTIVE clickdown CLIENTID clipbrd -CLIPCHILDREN -CLIPSIBLINGS closetest cloudconsole cloudvault @@ -338,7 +334,6 @@ dblclk Dcd DCOMMON DComposition -DDESHARE DDevice DEADCHAR debugtype @@ -385,6 +380,8 @@ DECNKM DECNRCM DECOM decommit +decommitted +decommitting DECPCCM DECPCTERM DECPS @@ -605,6 +602,7 @@ fte Ftm Fullscreens FUNCTIONCALL +Furia fuzzmain fuzzmap fuzzwrapper @@ -1228,7 +1226,6 @@ pcshell PCSHORT PCSR PCSTR -PCWCH PCWCHAR PCWSTR pdbs @@ -1346,7 +1343,6 @@ PSobject psp PSPCB psr -PSTR psz ptch ptsz @@ -1799,6 +1795,7 @@ uwa uwp uwu uxtheme +VADs Vanara vararg vclib @@ -1992,7 +1989,6 @@ yact YCast YCENTER YCount -yizz YLimit YPan YSubstantial diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 4776426a10e..56e3aeec10a 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -1254,6 +1254,12 @@ void ROW::SetScrollbarData(std::optional data) noexcept _promptData = data; } +void ROW::SetScrollbarData(std::nullopt_t) noexcept +{ + // Avoid copying the unused payload of an empty optional. + _promptData.reset(); +} + void ROW::StartPrompt() noexcept { if (!_promptData.has_value()) diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index d1157e06e3f..3d7499473ae 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -180,6 +180,7 @@ class ROW final const std::optional& GetScrollbarData() const noexcept; void SetScrollbarData(std::optional data) noexcept; + void SetScrollbarData(std::nullopt_t) noexcept; void StartPrompt() noexcept; void EndOutput(std::optional error) noexcept; diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 5acf2b324c5..f76d3fb2d96 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -34,6 +34,28 @@ constexpr bool allWhitespace(const std::wstring_view& text) noexcept return true; } +template +constexpr T* alignPointer(T* ptr, size_t alignment) noexcept +{ + return reinterpret_cast((reinterpret_cast(ptr) + alignment - 1) & ~(alignment - 1)); +} + +static size_t getPageSize() noexcept +{ + static std::atomic s_pageSize; + + const auto s = s_pageSize.load(std::memory_order_relaxed); + if (s) + { + return s; + } + + SYSTEM_INFO si; + GetSystemInfo(&si); + s_pageSize.store(si.dwPageSize, std::memory_order_relaxed); + return si.dwPageSize; +} + static std::atomic s_lastMutationIdInitialValue; // Routine Description: @@ -60,9 +82,6 @@ TextBuffer::TextBuffer(til::size screenBufferSize, _cursor{ cursorSize, *this }, _isActiveBuffer{ isActiveBuffer } { - // Guard against resizing the text buffer to 0 columns/rows, which would break being able to insert text. - screenBufferSize.width = std::max(screenBufferSize.width, 1); - screenBufferSize.height = std::max(screenBufferSize.height, 1); _reserve(screenBufferSize, defaultAttributes); } @@ -70,7 +89,7 @@ TextBuffer::~TextBuffer() { if (_buffer) { - _destroy(); + _destroy(_buffer.get()); } } @@ -91,8 +110,9 @@ TextBuffer::~TextBuffer() // memory usage from ~7MB down to just ~2MB at startup in the general case. void TextBuffer::_reserve(til::size screenBufferSize, const TextAttribute& defaultAttributes) { - const auto w = gsl::narrow(screenBufferSize.width); - const auto h = gsl::narrow(screenBufferSize.height); + // Guard against resizing the text buffer to 0 columns/rows, which would break being able to insert text. + const auto w = std::clamp(screenBufferSize.width, 1, 0xffff); + const auto h = std::clamp(screenBufferSize.height, 1, til::CoordTypeMax / 2); constexpr auto rowSize = ROW::CalculateRowSize(); const auto charsBufferSize = ROW::CalculateCharsBufferSize(w); @@ -103,7 +123,7 @@ void TextBuffer::_reserve(til::size screenBufferSize, const TextAttribute& defau // 65535*65535 cells would result in an allocSize of 8GiB. // --> Use uint64_t so that we can safely do our calculations even on x86. // We allocate 1 additional row, which will be used for GetScratchpadRow(). - const auto rowCount = ::base::strict_cast(h) + 1; + const auto rowCount = gsl::narrow_cast(h) + 1; const auto allocSize = gsl::narrow(rowCount * rowStride); // NOTE: Modifications to this block of code might have to be mirrored over to ResizeTraditional(). @@ -131,42 +151,105 @@ __declspec(noinline) void TextBuffer::_commit(const std::byte* row) { assert(row >= _commitWatermark); - const auto rowEnd = row + _bufferRowStride; - const auto remaining = gsl::narrow_cast(_bufferEnd - _commitWatermark); - const auto minimum = gsl::narrow_cast(rowEnd - _commitWatermark); - const auto ideal = minimum + _bufferRowStride * _commitReadAheadRowCount; - const auto size = std::min(remaining, ideal); + const auto current = gsl::narrow_cast(_commitWatermark - _buffer.get()); + const auto next = _commitChargeOptimize(row - _buffer.get()); + const auto size = next - current; + + if (next <= current) + { + assert(false); + return; + } THROW_LAST_ERROR_IF_NULL(VirtualAlloc(_commitWatermark, size, MEM_COMMIT, PAGE_READWRITE)); + _construct(_buffer.get() + next); +} - _construct(_commitWatermark + size); +uintptr_t TextBuffer::_commitChargeOptimize(uintptr_t rowOffset) noexcept +{ + const auto pageSize = getPageSize(); + const auto cap = gsl::narrow_cast(_bufferEnd - _buffer.get()); + // +N for chunked allocations, one of which accounts for the current row itself. + auto end = rowOffset + _bufferRowStride * _commitReadAheadRowCount; + // Align the end to the next page boundary to ensure we commit full pages. + end = (end + pageSize - 1) & ~(pageSize - 1); + // Ensure the returned value is aligned to row boundaries. + end = (end / _bufferRowStride) * _bufferRowStride; + // And ensure it's not past the end of the buffer of course. + end = std::min(end, cap); + return end; } -// Destructs and MEM_DECOMMITs all previously constructed ROWs. +// Destructs and MEM_DECOMMITs all rows between [rowsToKeep,_commitWatermark). // You can use this (or rather the Reset() method) to fully clear the TextBuffer. -void TextBuffer::_decommit() noexcept +void TextBuffer::_decommit(til::CoordType rowsToKeep) noexcept { - _destroy(); - VirtualFree(_buffer.get(), 0, MEM_DECOMMIT); - _commitWatermark = _buffer.get(); + rowsToKeep = std::clamp(rowsToKeep, 0, _height); + + const auto pageSize = getPageSize(); + const auto oldWatermark = _commitWatermark; + auto destructWatermark = _buffer.get(); + auto newWatermark = _buffer.get(); + auto pageWatermark = _buffer.get(); + + if (rowsToKeep > 0) + { + // +1 for the scratchpad row at offset 0. + const auto rows = gsl::narrow_cast(rowsToKeep) + 1; + + // Offset in bytes to the first row that we were asked to destroy. + // We must ensure that the offset is not past the end of the current _commitWatermark, + // since we don't want to finish with a watermark that's somehow larger than what we started with. + const auto commitBytes = gsl::narrow_cast(oldWatermark - _buffer.get()); + const auto byteOffset = std::min(commitBytes, rows * _bufferRowStride); + destructWatermark += byteOffset; + + newWatermark += std::min(commitBytes, _commitChargeOptimize(byteOffset)); + pageWatermark = alignPointer(newWatermark, pageSize); + } + + if (destructWatermark == oldWatermark) + { + return; + } + + // _destroy() takes care to check that the given pointer is valid. + _destroy(destructWatermark); + _commitWatermark = destructWatermark; + + // MEM_DECOMMIT the memory that we don't need anymore. + if (pageWatermark < oldWatermark) + { + // The analyzer can't know that we're intentionally decommitting memory here. +#pragma warning(suppress : 6250) // Calling 'VirtualFree' without the MEM_RELEASE flag may free memory but not address descriptors (VADs). This causes address space leaks. + VirtualFree(pageWatermark, oldWatermark - pageWatermark, MEM_DECOMMIT); + } + + _construct(newWatermark); + + // Several rows may have been reset back to whitespace, akin to a mutation. + _lastMutationId++; } // Constructs ROWs between [_commitWatermark,until). void TextBuffer::_construct(const std::byte* until) noexcept { + // _width has been validated to fit into uint16_t during reserve(). + const auto width = gsl::narrow_cast(_width); + for (; _commitWatermark < until; _commitWatermark += _bufferRowStride) { const auto row = reinterpret_cast(_commitWatermark); const auto chars = reinterpret_cast(_commitWatermark + _bufferOffsetChars); const auto indices = reinterpret_cast(_commitWatermark + _bufferOffsetCharOffsets); - std::construct_at(row, chars, indices, _width, _initialAttributes); + std::construct_at(row, chars, indices, width, _initialAttributes); } } -// Destructs ROWs between [_buffer,_commitWatermark). -void TextBuffer::_destroy() const noexcept +// Destructs ROWs between [it,_commitWatermark). +void TextBuffer::_destroy(std::byte* it) const noexcept { - for (auto it = _buffer.get(); it < _commitWatermark; it += _bufferRowStride) + for (; it < _commitWatermark; it += _bufferRowStride) { std::destroy_at(reinterpret_cast(it)); } @@ -805,71 +888,74 @@ void TextBuffer::_SetFirstRowIndex(const til::CoordType FirstRowIndex) noexcept void TextBuffer::ScrollRows(const til::CoordType firstRow, til::CoordType size, const til::CoordType delta) { - if (delta == 0) + size = std::clamp(size, 0, _height); + + const auto shift = (delta % _height + _height) % _height; + if (size == 0 || shift == 0) { return; } - // Since the for() loop uses !=, we must ensure that size is positive. - // A negative size doesn't make any sense anyways. - size = std::max(0, size); - - til::CoordType y = 0; - til::CoordType end = 0; - til::CoordType step = 0; - - if (delta < 0) - { - // The layout is like this: - // delta is -2, size is 3, firstRow is 5 - // We want 3 rows from 5 (5, 6, and 7) to move up 2 spots. - // --- (storage) ---- - // | 0 begin - // | 1 - // | 2 - // | 3 A. firstRow + delta (because delta is negative) - // | 4 - // | 5 B. firstRow - // | 6 - // | 7 - // | 8 C. firstRow + size - // | 9 - // | 10 - // | 11 - // - end - // We want B to slide up to A (the negative delta) and everything from [B,C) to slide up with it. - y = firstRow; - end = firstRow + size; - step = 1; - } - else + // This clever algorithm is sometimes called the "juggling algorithm" for array rotation. + // It's not actually novel. Both the Rust stdlib and Microsoft STL use it (and many other). + // What's weird is that none list an authoritative description for it. Here's one contender: + // Carlo A. Furia, Rotation of Sequences: Algorithms and Proofs + // ยง4.4: Rotation by modular visit + // https://arxiv.org/html/1406.5453v3#S4.SS4 + // + // This implementation is a bit special in that it does partial rotations of just `size` elements. + + const auto height = _height; + const auto start = (firstRow % height + height) % height; + const auto reverseShift = height - shift; + const auto get = [=, this](const til::CoordType y) -> ROW& { + return GetMutableRowByOffset((start + y) % height); + }; + const auto copy = [this](ROW& src, ROW& dst) { + dst.CopyFrom(src); + dst.SetScrollbarData(src.GetScrollbarData()); + src.SetScrollbarData(std::nullopt); + ImageSlice::CopyRow(src, dst); + }; + const auto copyByIndex = [&](const til::CoordType src, const til::CoordType dst) { + copy(get(src), get(dst)); + }; + + // Chains have a safe place to start: a destination that isn't also a source. + // Follow it backwards, copying into the slot we just read. + const auto chainEnd = std::min(size, reverseShift); + for (til::CoordType last = std::max(0, size - shift); last < chainEnd; ++last) { - // The layout is like this: - // delta is 2, size is 3, firstRow is 5 - // We want 3 rows from 5 (5, 6, and 7) to move down 2 spots. - // --- (storage) ---- - // | 0 begin - // | 1 - // | 2 - // | 3 - // | 4 - // | 5 A. firstRow - // | 6 - // | 7 - // | 8 B. firstRow + size - // | 9 - // | 10 C. firstRow + size + delta - // | 11 - // - end - // We want B-1 to slide down to C-1 (the positive delta) and everything from [A, B) to slide down with it. - y = firstRow + size - 1; - end = firstRow - 1; - step = -1; + auto src = last; + auto dst = last + shift; + do + { + copyByIndex(src, dst); + dst = src; + src = (src + reverseShift) % height; + } while (src < size); } - for (; y != end; y += step) + // Cycles have no safe starting slot, so save one row in scratch. A cycle starting + // at `r` survives the partial copy iff `r + height - gcd(height, shift) < size`. + const auto cycleCount = size - height + std::gcd(height, shift); + if (cycleCount > 0) { - CopyRow(y, y + delta, *this); + auto& scratch = GetScratchpadRow(); + + for (til::CoordType first = 0; first < cycleCount; ++first) + { + copy(get(first), scratch); + auto dst = first; + auto src = (dst + reverseShift) % height; + while (src != first) + { + copyByIndex(src, dst); + dst = src; + src = (src + reverseShift) % height; + } + copy(scratch, get(dst)); + } } } @@ -989,7 +1075,7 @@ til::point TextBuffer::BufferToScreenPosition(const til::point position) const // and the default current color attributes void TextBuffer::Reset() noexcept { - _decommit(); + _decommit(0); _initialAttributes = _currentAttributes; } @@ -999,34 +1085,19 @@ void TextBuffer::Reset() noexcept void TextBuffer::ClearScrollback(const til::CoordType newFirstRow, const til::CoordType rowsToKeep) { // We're already at the top? don't clear anything. There's no scrollback. - if (newFirstRow <= 0) + if (newFirstRow < 0) { return; } - // The new viewport should keep 0 rows? Then just reset everything. - if (rowsToKeep <= 0) - { - _decommit(); - return; - } - - ClearMarksInRange(til::point{ 0, 0 }, til::point{ _width, std::max(0, newFirstRow - 1) }); - // Our goal is to move the viewport to the absolute start of the underlying memory buffer so that we can - // MEM_DECOMMIT the remaining memory. _firstRow is used to make the TextBuffer behave like a circular buffer. - // The newFirstRow parameter is relative to the _firstRow. The trick to get the content to the absolute start - // is to simply add _firstRow ourselves and then reset it to 0. This causes ScrollRows() to write into - // the absolute start while reading from relative coordinates. This works because GetRowByOffset() - // operates modulo the buffer height and so the possibly-too-large startAbsolute won't be an issue. - const auto startAbsolute = _firstRow + newFirstRow; + // Move the retained rows to the start of the allocation so the remaining pages can be decommitted. + const auto startAbsolute = (_firstRow + newFirstRow) % _height; + const auto keep = std::clamp(rowsToKeep, 0, _height); _firstRow = 0; - ScrollRows(startAbsolute, rowsToKeep, -startAbsolute); + ScrollRows(startAbsolute, keep, -startAbsolute); - const auto end = _estimateOffsetOfLastCommittedRow(); - for (auto y = rowsToKeep; y <= end; ++y) - { - GetMutableRowByOffset(y).Reset(_initialAttributes); - } + // Decommit any remaining pages that are no longer needed. + _decommit(keep); } // Routine Description: @@ -1741,7 +1812,7 @@ std::vector TextBuffer::GetTextSpans(til::point start, til::poi const auto rects = GetTextRects(start, end, /*blockSelection*/ true, bufferCoordinates); textSpans.reserve(rects.size()); - for (auto rect : rects) + for (const auto& rect : rects) { const til::point first = { rect.left, rect.top }; const til::point second = { rect.right, rect.bottom }; @@ -3040,7 +3111,7 @@ void TextBuffer::RemoveHyperlinkFromMap(uint16_t id) noexcept // - The custom ID if there was one, empty string otherwise std::wstring TextBuffer::GetCustomIdFromId(uint16_t id) const { - for (auto customIdPair : _hyperlinkCustomIdMap) + for (const auto& customIdPair : _hyperlinkCustomIdMap) { if (customIdPair.second == id) { @@ -3236,7 +3307,6 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, bool startedPrompt = false; bool startedCommand = false; bool startedOutput = false; - MarkKind lastMarkKind = MarkKind::Output; const auto endThisMark = [&](auto x, auto y) { if (startedOutput) @@ -3263,7 +3333,7 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, // Output attribute. const auto& row = GetRowByOffset(y); - const auto runs = row.Attributes().runs(); + const auto& runs = row.Attributes().runs(); x = 0; for (const auto& [attr, length] : runs) { @@ -3306,8 +3376,6 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, endThisMark(lastMarkedText.x, lastMarkedText.y); } - // Otherwise, we've changed from any state -> any state, and it doesn't really matter. - lastMarkKind = markKind; } // advance to next run of text x = nextX; @@ -3340,7 +3408,7 @@ std::wstring TextBuffer::_commandForRow(const til::CoordType rowOffset, // Command attributes. Collect up all of those, till we get to the next // Output attribute. const auto& row = GetRowByOffset(y); - const auto runs = row.Attributes().runs(); + const auto& runs = row.Attributes().runs(); auto x = 0; for (const auto& [attr, length] : runs) { diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 1923b22be05..3e0896d6d10 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -316,9 +316,10 @@ class TextBuffer final private: void _reserve(til::size screenBufferSize, const TextAttribute& defaultAttributes); void _commit(const std::byte* row); - void _decommit() noexcept; + uintptr_t _commitChargeOptimize(uintptr_t rowOffset) noexcept; + void _decommit(til::CoordType keep) noexcept; void _construct(const std::byte* until) noexcept; - void _destroy() const noexcept; + void _destroy(std::byte* it) const noexcept; ROW& _getRowByOffsetDirect(size_t offset); ROW& _getRow(til::CoordType y) const; til::CoordType _estimateOffsetOfLastCommittedRow() const noexcept; @@ -400,9 +401,9 @@ class TextBuffer final size_t _bufferOffsetChars = 0; size_t _bufferOffsetCharOffsets = 0; // The width of the buffer in columns. - uint16_t _width = 0; + til::CoordType _width = 0; // The height of the buffer in rows, excluding the scratchpad row. - uint16_t _height = 0; + til::CoordType _height = 0; TextAttribute _currentAttributes; til::CoordType _firstRow = 0; // indexes top row (not necessarily 0) diff --git a/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp b/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp index c6b45bd5479..a0a58a8b1a3 100644 --- a/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/TerminalBufferTests.cpp @@ -35,6 +35,7 @@ class TerminalCoreUnitTests::TerminalBufferTests final TEST_CLASS(TerminalBufferTests); TEST_METHOD(TestSimpleBufferWriting); + TEST_METHOD(TestClearScreenAndScrollback); TEST_METHOD(TestWrappingCharByChar); TEST_METHOD(TestWrappingALongString); @@ -96,6 +97,71 @@ void TerminalBufferTests::TestSimpleBufferWriting() TestUtils::VerifyExpectedString(termTb, L"Hello World", { 0, 0 }); } +void TerminalBufferTests::TestClearScreenAndScrollback() +{ + static constexpr auto virtualSize = [](const void* address) -> int { + MEMORY_BASIC_INFORMATION info; + const auto r = VirtualQuery(address, &info, sizeof(info)); + return r ? gsl::narrow_cast(info.RegionSize) : 0; + }; + static constexpr auto virtualState = [](const void* address) -> int { + MEMORY_BASIC_INFORMATION info; + const auto r = VirtualQuery(address, &info, sizeof(info)); + return r ? gsl::narrow_cast(info.State) : 0; + }; + + term->Create({ TerminalViewWidth, TerminalViewHeight }, 4096, *emptyRenderer); + auto& buffer = *term->_mainBuffer; + auto& stateMachine = *term->_stateMachine; + + // Rotate by 17 so the retained viewport wraps around + // the allocation and overlaps its destination. + stateMachine.ProcessString(std::wstring(17, L'\n')); + stateMachine.ProcessString(L"\x1b]133;A\aold\x1b]133;D;1\a"); + stateMachine.ProcessString(std::wstring(buffer.TotalRowCount() - 1, L'\n')); + stateMachine.ProcessString(L"\x1b[H\x1b]133;A\aa\x1b]133;D;0\a\x1b[6;1H\x1b]133;A\ab"); + + VERIFY_ARE_EQUAL(17, buffer.GetFirstRowIndex()); + // These offsets map to the first and last physical rows after the rotation. + const auto firstRow = &buffer.GetRowByOffset(-17); + const auto lastRow = &buffer.GetRowByOffset(-18); + VERIFY_ARE_EQUAL(MEM_COMMIT, virtualState(lastRow)); + + // Erasing the scrollback should rotate the buffer + // and decommit the now unused memory. + stateMachine.ProcessString(L"\x1b[3J"); + + VERIFY_ARE_EQUAL(0, term->GetViewport().Top()); + VERIFY_ARE_EQUAL(0, buffer.GetFirstRowIndex()); + TestUtils::VerifyExpectedString(buffer, L"a", { 0, 0 }); + TestUtils::VerifyExpectedString(buffer, L"b", { 0, 5 }); + const auto marks = buffer.GetMarkRows(); + VERIFY_ARE_EQUAL(size_t{ 2 }, marks.size()); + VERIFY_ARE_EQUAL(0, marks[0].row); + VERIFY_ARE_EQUAL(MarkCategory::Success, marks[0].data.category); + VERIFY_IS_TRUE(marks[0].data.exitCode.has_value()); + VERIFY_ARE_EQUAL(0u, *marks[0].data.exitCode); + VERIFY_ARE_EQUAL(5, marks[1].row); + VERIFY_ARE_EQUAL(MarkCategory::Prompt, marks[1].data.category); + VERIFY_IS_FALSE(marks[1].data.exitCode.has_value()); + + VERIFY_ARE_EQUAL(MEM_RESERVE, virtualState(lastRow)); + VERIFY_ARE_EQUAL(MEM_COMMIT, virtualState(firstRow)); + VERIFY_IS_LESS_THAN(virtualSize(firstRow), 1024 * 1024); + + stateMachine.ProcessString(L"\x1b[H\x1b[2J\x1b[3J"); + + VERIFY_ARE_EQUAL(0, term->GetViewport().Top()); + VERIFY_IS_TRUE(buffer.GetMarkRows().empty()); + for (auto y = 0; y < TerminalViewHeight; ++y) + { + VERIFY_ARE_EQUAL(0, buffer.GetRowByOffset(y).MeasureRight()); + } + + stateMachine.ProcessString(L"new text"); + TestUtils::VerifyExpectedString(buffer, L"new text", { 0, 0 }); +} + void TerminalBufferTests::TestWrappingCharByChar() { auto& termTb = *term->_mainBuffer; diff --git a/src/common.build.pre.props b/src/common.build.pre.props index b726c916f19..3e81f97128c 100644 --- a/src/common.build.pre.props +++ b/src/common.build.pre.props @@ -129,12 +129,10 @@ C26456: Operator 'A' hides a non-virtual operator 'B' (c.128) I think these rules are for when you fully bought into OOP? We didn't and it breaks WRL and large parts of conhost code. - C26478: Don't use std::move on constant variables. (es.56). - This diagnostic is broken in VS 17.7 which our CI currently uses. It's fixed in 17.8. - C26494: Variable 'index' is uninitialized. Always initialize an object (type. 5). - This diagnostic is broken in VS 17.7 which our CI currently uses. It's fixed in 17.8. + C26462: The value pointed to by 'variable' is assigned only once, mark it as a pointer to const (con.4). + Way too pedantic and breaks trivial `const auto p = some_mut_ptr;`. --> - 4201;4312;5105;26434;26445;26456;26478;26494;%(DisableSpecificWarnings) + 4201;4312;26434;26445;26456;26462;%(DisableSpecificWarnings) _WINDOWS;EXTERNAL_BUILD;_SILENCE_STDEXT_ARR_ITERS_DEPRECATION_WARNING;%(PreprocessorDefinitions) true precomp.h diff --git a/src/inc/til/rle.h b/src/inc/til/rle.h index 7f80aba6d3b..200101d1318 100644 --- a/src/inc/til/rle.h +++ b/src/inc/til/rle.h @@ -395,16 +395,27 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // This method gives access to the raw run length encoded array // and allows users of this class to iterate over those. - const container& runs() const noexcept + const container& runs() const& noexcept { return _runs; } - container& runs() noexcept + container& runs() & noexcept { return _runs; } + container runs() && noexcept + { + auto runs = std::move(_runs); + // See basic_rle(basic_rle&&) for why this is necessary. + if (_runs.empty()) + { + _total_length = 0; + } + return runs; + } + // Get the value at the position const_reference at(size_type position) const { From 05a7892306182915c7ebb3a0d55edc2e9b5f0268 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 6 Nov 2026 14:56:53 +0100 Subject: [PATCH 2/2] Ignore C26494 --- src/common.build.pre.props | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/common.build.pre.props b/src/common.build.pre.props index 3e81f97128c..c6662176be6 100644 --- a/src/common.build.pre.props +++ b/src/common.build.pre.props @@ -130,9 +130,11 @@ I think these rules are for when you fully bought into OOP? We didn't and it breaks WRL and large parts of conhost code. C26462: The value pointed to by 'variable' is assigned only once, mark it as a pointer to const (con.4). - Way too pedantic and breaks trivial `const auto p = some_mut_ptr;`. + Breaks trivial `const auto p = some_mut_ptr;`, because `p` is not `const*`. + C26494: Variable 'index' is uninitialized. Always initialize an object (type. 5). + Breaks provably correct code such as `int a; a = 5;`. --> - 4201;4312;26434;26445;26456;26462;%(DisableSpecificWarnings) + 4201;4312;26434;26445;26456;26462;26494;%(DisableSpecificWarnings) _WINDOWS;EXTERNAL_BUILD;_SILENCE_STDEXT_ARR_ITERS_DEPRECATION_WARNING;%(PreprocessorDefinitions) true precomp.h