From 3ddadf48bace77dca3f96a6a9265c6602aee3237 Mon Sep 17 00:00:00 2001 From: YexuanXiao Date: Tue, 17 Dec 2024 12:02:33 +0800 Subject: [PATCH 1/8] Update GSL header names, remove dependencies on unstable STL extensions, and fix non-standard compliant code --- src/host/VtIo.cpp | 19 ++++++++++++++----- src/inc/LibraryIncludes.h | 5 ++++- src/renderer/atlas/AtlasEngine.api.cpp | 1 + src/renderer/atlas/pch.h | 4 ++++ src/renderer/gdi/gdirenderer.hpp | 2 +- src/renderer/gdi/paint.cpp | 2 +- src/server/WaitBlock.h | 4 ++-- src/terminal/adapter/SixelParser.hpp | 2 +- src/types/colorTable.cpp | 2 +- src/types/inc/colorTable.hpp | 2 +- 10 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp index d64d142d210..5475954ec3a 100644 --- a/src/host/VtIo.cpp +++ b/src/host/VtIo.cpp @@ -567,17 +567,26 @@ void VtIo::Writer::WriteUTF16(std::wstring_view str) const // C++23's resize_and_overwrite is too valuable to not use. // It reduce the CPU overhead by roughly half. -#if !defined(_HAS_CXX23) || !_HAS_CXX23 -#define resize_and_overwrite _Resize_and_overwrite +#if !defined(__cpp_lib_string_resize_and_overwrite) +#if defined(_MSVC_STL_UPDATE) + // NOTE: Throwing inside resize_and_overwrite invokes undefined behavior. + _io->_back._Resize_and_overwrite(totalUTF8Cap, [&](char* buf, const size_t) noexcept { + const auto len = WideCharToMultiByte(CP_UTF8, 0, str.data(), gsl::narrow_cast(incomingUTF16Len), buf + existingUTF8Len, gsl::narrow_cast(incomingUTF8Cap), nullptr, nullptr); + return existingUTF8Len + std::max(0, len); + }); +#else + auto& back = _io->_back; + back.resize(totalUTF8Cap); + const auto len = WideCharToMultiByte(CP_UTF8, 0, str.data(), gsl::narrow_cast(incomingUTF16Len), back.data() + existingUTF8Len, gsl::narrow_cast(incomingUTF8Cap), nullptr, nullptr); + back.resize(existingUTF8Len + std::max(0, len)); #endif - +#else // NOTE: Throwing inside resize_and_overwrite invokes undefined behavior. _io->_back.resize_and_overwrite(totalUTF8Cap, [&](char* buf, const size_t) noexcept { const auto len = WideCharToMultiByte(CP_UTF8, 0, str.data(), gsl::narrow_cast(incomingUTF16Len), buf + existingUTF8Len, gsl::narrow_cast(incomingUTF8Cap), nullptr, nullptr); return existingUTF8Len + std::max(0, len); }); - -#undef resize_and_overwrite +#endif } // When DISABLE_NEWLINE_AUTO_RETURN is not set (Bad! Don't do it!) we'll do newline translation for you. diff --git a/src/inc/LibraryIncludes.h b/src/inc/LibraryIncludes.h index a80e351ffeb..3b6d14d02ac 100644 --- a/src/inc/LibraryIncludes.h +++ b/src/inc/LibraryIncludes.h @@ -65,8 +65,11 @@ // GSL // Block GSL Multi Span include because it both has C++17 deprecated iterators // and uses the C-namespaced "max" which conflicts with Windows definitions. -#include +#if __has_include() #include +#else +#include +#endif #include // CppCoreCheck diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index a80ae930ba9..5528968f9c1 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -7,6 +7,7 @@ #include "Backend.h" #include "../../buffer/out/textBuffer.hpp" #include "../base/FontCache.h" +#include // #### NOTE #### // If you see any code in here that contains "_r." you might be seeing a race condition. diff --git a/src/renderer/atlas/pch.h b/src/renderer/atlas/pch.h index 240af13275b..11a4735f3ef 100644 --- a/src/renderer/atlas/pch.h +++ b/src/renderer/atlas/pch.h @@ -24,7 +24,11 @@ #include #include +#if __has_include() #include +#else +#include +#endif #include #include #include diff --git a/src/renderer/gdi/gdirenderer.hpp b/src/renderer/gdi/gdirenderer.hpp index d88f10de784..c34efdec593 100644 --- a/src/renderer/gdi/gdirenderer.hpp +++ b/src/renderer/gdi/gdirenderer.hpp @@ -165,7 +165,7 @@ namespace Microsoft::Console::Render // It's important the pool is first so it can be given to the others on construction. std::pmr::unsynchronized_pool_resource _pool; std::pmr::vector _polyStrings; - std::pmr::vector> _polyWidths; + std::pmr::vector> _polyWidths; std::vector _imageMask; diff --git a/src/renderer/gdi/paint.cpp b/src/renderer/gdi/paint.cpp index 58eca36f843..e07fd9f4b0a 100644 --- a/src/renderer/gdi/paint.cpp +++ b/src/renderer/gdi/paint.cpp @@ -419,7 +419,7 @@ bool GdiEngine::FontHasWesternScript(HDC hdc) pPolyTextLine->rcl.top = pPolyTextLine->y + topOffset; pPolyTextLine->rcl.right = pPolyTextLine->rcl.left + (til::CoordType)cchCharWidths; pPolyTextLine->rcl.bottom = pPolyTextLine->y + coordFontSize.height - bottomOffset; - pPolyTextLine->pdx = polyWidth.data(); + pPolyTextLine->pdx = reinterpret_cast(polyWidth.data()); if (trimLeft) { diff --git a/src/server/WaitBlock.h b/src/server/WaitBlock.h index d1964394769..560eaee91f9 100644 --- a/src/server/WaitBlock.h +++ b/src/server/WaitBlock.h @@ -42,10 +42,10 @@ class ConsoleWaitBlock _In_ IWaitRoutine* const pWaiter); ConsoleWaitQueue* const _pProcessQueue; - std::_List_const_iterator>> _itProcessQueue; + typename std::list::const_iterator _itProcessQueue; ConsoleWaitQueue* const _pObjectQueue; - std::_List_const_iterator>> _itObjectQueue; + typename std::list::const_iterator _itObjectQueue; CONSOLE_API_MSG _WaitReplyMessage; diff --git a/src/terminal/adapter/SixelParser.hpp b/src/terminal/adapter/SixelParser.hpp index 1966228d77f..ea97783fec2 100644 --- a/src/terminal/adapter/SixelParser.hpp +++ b/src/terminal/adapter/SixelParser.hpp @@ -104,7 +104,7 @@ namespace Microsoft::Console::VirtualTerminal size_t _colorsUsed = 0; size_t _colorsAvailable = 0; bool _colorTableChanged = false; - IndexedPixel _foregroundPixel = 0; + IndexedPixel _foregroundPixel = {}; void _initImageBuffer(); void _resizeImageBuffer(const til::CoordType requiredHeight); diff --git a/src/types/colorTable.cpp b/src/types/colorTable.cpp index 0a67fe510c7..3eb3350d986 100644 --- a/src/types/colorTable.cpp +++ b/src/types/colorTable.cpp @@ -255,7 +255,7 @@ void Utils::InitializeVT340ColorTable(const std::span table) noexcept // - table: a color table to be filled // Return Value: // - -constexpr void Utils::InitializeExtendedColorTable(const std::span table, const bool monochrome) noexcept +void Utils::InitializeExtendedColorTable(const std::span table, const bool monochrome) noexcept { if (table.size() >= 256) { diff --git a/src/types/inc/colorTable.hpp b/src/types/inc/colorTable.hpp index 8e2187dc98b..7d659c9e003 100644 --- a/src/types/inc/colorTable.hpp +++ b/src/types/inc/colorTable.hpp @@ -15,7 +15,7 @@ namespace Microsoft::Console::Utils void InitializeColorTable(const std::span table) noexcept; void InitializeANSIColorTable(const std::span table) noexcept; void InitializeVT340ColorTable(const std::span table) noexcept; - constexpr void InitializeExtendedColorTable(const std::span table, const bool monochrome = false) noexcept; + void InitializeExtendedColorTable(const std::span table, const bool monochrome = false) noexcept; std::span CampbellColorTable() noexcept; std::optional ColorFromXOrgAppColorName(const std::wstring_view wstr) noexcept; From 40e962cf567513bdaeb42c374cfc1a4cc4aef5f6 Mon Sep 17 00:00:00 2001 From: YexuanXiao Date: Tue, 17 Dec 2024 12:36:55 +0800 Subject: [PATCH 2/8] Try to suppress false positive warnings. --- src/types/colorTable.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/types/colorTable.cpp b/src/types/colorTable.cpp index 3eb3350d986..c86a9df8959 100644 --- a/src/types/colorTable.cpp +++ b/src/types/colorTable.cpp @@ -247,6 +247,9 @@ void Utils::InitializeVT340ColorTable(const std::span table) noexcept } } +#pragma warning(push) +#pragma warning(disable : 26497) // This is a false positive in audit mode. + // Function Description: // - Fill color table entries from 16 to 255 with the default colors used by // modern terminals. This includes a range of colors from a 6x6x6 color cube @@ -279,6 +282,8 @@ void Utils::InitializeExtendedColorTable(const std::span table, const } } +#pragma warning(pop) + #pragma warning(push) #pragma warning(disable : 26447) // This is a false positive. From cf2bd10adab38af6ee6ea2173db84814751c40f2 Mon Sep 17 00:00:00 2001 From: YexuanXiao Date: Tue, 17 Dec 2024 15:01:36 +0800 Subject: [PATCH 3/8] Fix TIL's dependency on GSL. --- src/inc/LibraryIncludes.h | 1 + src/renderer/atlas/pch.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/inc/LibraryIncludes.h b/src/inc/LibraryIncludes.h index 3b6d14d02ac..ffdfd82636a 100644 --- a/src/inc/LibraryIncludes.h +++ b/src/inc/LibraryIncludes.h @@ -71,6 +71,7 @@ #include #endif #include +#include // CppCoreCheck #include diff --git a/src/renderer/atlas/pch.h b/src/renderer/atlas/pch.h index 11a4735f3ef..ab60082363d 100644 --- a/src/renderer/atlas/pch.h +++ b/src/renderer/atlas/pch.h @@ -30,6 +30,7 @@ #include #endif #include +#include #include #include #include From 92eb45124cb9598132b692f227008ef1cca609df Mon Sep 17 00:00:00 2001 From: YexuanXiao Date: Tue, 17 Dec 2024 15:14:19 +0800 Subject: [PATCH 4/8] Fix TIL's dependency on GSL. --- src/inc/LibraryIncludes.h | 2 ++ src/renderer/atlas/pch.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/inc/LibraryIncludes.h b/src/inc/LibraryIncludes.h index ffdfd82636a..55549b1b7b7 100644 --- a/src/inc/LibraryIncludes.h +++ b/src/inc/LibraryIncludes.h @@ -71,7 +71,9 @@ #include #endif #include +#if __has_include() #include +#endif // CppCoreCheck #include diff --git a/src/renderer/atlas/pch.h b/src/renderer/atlas/pch.h index ab60082363d..57f889f61ea 100644 --- a/src/renderer/atlas/pch.h +++ b/src/renderer/atlas/pch.h @@ -30,7 +30,9 @@ #include #endif #include +#if __has_include() #include +#endif #include #include #include From e39d5ff9ae51533244bec1ea3d26ed23814253c3 Mon Sep 17 00:00:00 2001 From: YexuanXiao Date: Wed, 18 Dec 2024 05:30:46 +0800 Subject: [PATCH 5/8] Replace string with vector, not used as character nor need the null terminator --- src/renderer/gdi/gdirenderer.hpp | 2 +- src/renderer/gdi/paint.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderer/gdi/gdirenderer.hpp b/src/renderer/gdi/gdirenderer.hpp index c34efdec593..d8bac221314 100644 --- a/src/renderer/gdi/gdirenderer.hpp +++ b/src/renderer/gdi/gdirenderer.hpp @@ -165,7 +165,7 @@ namespace Microsoft::Console::Render // It's important the pool is first so it can be given to the others on construction. std::pmr::unsynchronized_pool_resource _pool; std::pmr::vector _polyStrings; - std::pmr::vector> _polyWidths; + std::pmr::vector> _polyWidths; std::vector _imageMask; diff --git a/src/renderer/gdi/paint.cpp b/src/renderer/gdi/paint.cpp index e07fd9f4b0a..86db0de9400 100644 --- a/src/renderer/gdi/paint.cpp +++ b/src/renderer/gdi/paint.cpp @@ -362,7 +362,7 @@ bool GdiEngine::FontHasWesternScript(HDC hdc) polyString.back() &= softFontCharMask; polyWidth.push_back(gsl::narrow(cluster.GetColumns()) * coordFontSize.width); cchCharWidths += polyWidth.back(); - polyWidth.append(text.size() - 1, 0); + polyWidth.resize(polyWidth.size() + text.size() - 1); } // Detect and convert for raster font... @@ -419,7 +419,7 @@ bool GdiEngine::FontHasWesternScript(HDC hdc) pPolyTextLine->rcl.top = pPolyTextLine->y + topOffset; pPolyTextLine->rcl.right = pPolyTextLine->rcl.left + (til::CoordType)cchCharWidths; pPolyTextLine->rcl.bottom = pPolyTextLine->y + coordFontSize.height - bottomOffset; - pPolyTextLine->pdx = reinterpret_cast(polyWidth.data()); + pPolyTextLine->pdx = polyWidth.data(); if (trimLeft) { From c2a4a145e179664bbc176bdfeccd4f15679501a0 Mon Sep 17 00:00:00 2001 From: YexuanXiao Date: Wed, 18 Dec 2024 05:34:01 +0800 Subject: [PATCH 6/8] Move #include to pch.h. --- src/renderer/atlas/AtlasEngine.api.cpp | 1 - src/renderer/atlas/pch.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index 5528968f9c1..a80ae930ba9 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -7,7 +7,6 @@ #include "Backend.h" #include "../../buffer/out/textBuffer.hpp" #include "../base/FontCache.h" -#include // #### NOTE #### // If you see any code in here that contains "_r." you might be seeing a race condition. diff --git a/src/renderer/atlas/pch.h b/src/renderer/atlas/pch.h index 57f889f61ea..71b12982c69 100644 --- a/src/renderer/atlas/pch.h +++ b/src/renderer/atlas/pch.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include From a199a35fc1127c675499e2f7c9e852f7e2c63235 Mon Sep 17 00:00:00 2001 From: YexuanXiao Date: Wed, 18 Dec 2024 05:39:16 +0800 Subject: [PATCH 7/8] Revert GSL header files. --- src/inc/LibraryIncludes.h | 7 ------- src/renderer/atlas/pch.h | 7 ------- 2 files changed, 14 deletions(-) diff --git a/src/inc/LibraryIncludes.h b/src/inc/LibraryIncludes.h index 55549b1b7b7..6a2c1a1393b 100644 --- a/src/inc/LibraryIncludes.h +++ b/src/inc/LibraryIncludes.h @@ -65,15 +65,8 @@ // GSL // Block GSL Multi Span include because it both has C++17 deprecated iterators // and uses the C-namespaced "max" which conflicts with Windows definitions. -#if __has_include() #include -#else -#include -#endif #include -#if __has_include() -#include -#endif // CppCoreCheck #include diff --git a/src/renderer/atlas/pch.h b/src/renderer/atlas/pch.h index 71b12982c69..7e9109010b9 100644 --- a/src/renderer/atlas/pch.h +++ b/src/renderer/atlas/pch.h @@ -25,15 +25,8 @@ #include #include -#if __has_include() #include -#else -#include -#endif #include -#if __has_include() -#include -#endif #include #include #include From 01a24700abac14517bee49de7b7f5fdd5bd0deef Mon Sep 17 00:00:00 2001 From: YexuanXiao Date: Wed, 18 Dec 2024 06:23:58 +0800 Subject: [PATCH 8/8] Simplify code and reject old STL. --- src/host/VtIo.cpp | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp index 5475954ec3a..1b8a8591d20 100644 --- a/src/host/VtIo.cpp +++ b/src/host/VtIo.cpp @@ -567,26 +567,18 @@ void VtIo::Writer::WriteUTF16(std::wstring_view str) const // C++23's resize_and_overwrite is too valuable to not use. // It reduce the CPU overhead by roughly half. -#if !defined(__cpp_lib_string_resize_and_overwrite) -#if defined(_MSVC_STL_UPDATE) - // NOTE: Throwing inside resize_and_overwrite invokes undefined behavior. - _io->_back._Resize_and_overwrite(totalUTF8Cap, [&](char* buf, const size_t) noexcept { - const auto len = WideCharToMultiByte(CP_UTF8, 0, str.data(), gsl::narrow_cast(incomingUTF16Len), buf + existingUTF8Len, gsl::narrow_cast(incomingUTF8Cap), nullptr, nullptr); - return existingUTF8Len + std::max(0, len); - }); -#else - auto& back = _io->_back; - back.resize(totalUTF8Cap); - const auto len = WideCharToMultiByte(CP_UTF8, 0, str.data(), gsl::narrow_cast(incomingUTF16Len), back.data() + existingUTF8Len, gsl::narrow_cast(incomingUTF8Cap), nullptr, nullptr); - back.resize(existingUTF8Len + std::max(0, len)); +#if !defined(__cpp_lib_string_resize_and_overwrite) && _MSVC_STL_UPDATE >= 202111L +#define resize_and_overwrite _Resize_and_overwrite +#elif !defined(__cpp_lib_string_resize_and_overwrite) +#error "rely on resize_and_overwrite" #endif -#else // NOTE: Throwing inside resize_and_overwrite invokes undefined behavior. _io->_back.resize_and_overwrite(totalUTF8Cap, [&](char* buf, const size_t) noexcept { const auto len = WideCharToMultiByte(CP_UTF8, 0, str.data(), gsl::narrow_cast(incomingUTF16Len), buf + existingUTF8Len, gsl::narrow_cast(incomingUTF8Cap), nullptr, nullptr); return existingUTF8Len + std::max(0, len); }); -#endif + +#undef resize_and_overwrite } // When DISABLE_NEWLINE_AUTO_RETURN is not set (Bad! Don't do it!) we'll do newline translation for you.