Remove dependencies on unstable MSVC/STL extensions, #18334
Conversation
…ns, and fix non-standard compliant code
| @@ -42,10 +42,10 @@ class ConsoleWaitBlock | |||
| _In_ IWaitRoutine* const pWaiter); | |||
|
|
|||
| ConsoleWaitQueue* const _pProcessQueue; | |||
| std::_List_const_iterator<std::_List_val<std::_List_simple_types<ConsoleWaitBlock*>>> _itProcessQueue; | |||
| typename std::list<ConsoleWaitBlock*>::const_iterator _itProcessQueue; | |||
There was a problem hiding this comment.
Remove the use of unstable STL internal names.
| @@ -104,7 +104,7 @@ namespace Microsoft::Console::VirtualTerminal | |||
| size_t _colorsUsed = 0; | |||
| size_t _colorsAvailable = 0; | |||
| bool _colorTableChanged = false; | |||
| IndexedPixel _foregroundPixel = 0; | |||
| IndexedPixel _foregroundPixel = {}; | |||
There was a problem hiding this comment.
The struct has two members, and allowing the use of = 0 initialization might be an MSVC extension or a bug.
| @@ -255,7 +255,7 @@ void Utils::InitializeVT340ColorTable(const std::span<COLORREF> table) noexcept | |||
| // - table: a color table to be filled | |||
| // Return Value: | |||
| // - <none> | |||
| constexpr void Utils::InitializeExtendedColorTable(const std::span<COLORREF> table, const bool monochrome) noexcept | |||
| void Utils::InitializeExtendedColorTable(const std::span<COLORREF> table, const bool monochrome) noexcept | |||
There was a problem hiding this comment.
Removed the strange constexpr as it might cause the function definition to be discarded, leading to missing symbols.
| @@ -567,10 +567,11 @@ 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 | |||
| #if !defined(__cpp_lib_string_resize_and_overwrite) && _MSVC_STL_UPDATE >= 202111L | |||
There was a problem hiding this comment.
Using feature test macros instead of MSVC private macros; this will no longer be necessary after upgrading to C++23. The macro _MSVC_STL_UPDATE is documented.
| @@ -13,6 +13,7 @@ | |||
| #include <span> | |||
| #include <string_view> | |||
| #include <vector> | |||
| #include <cassert> | |||
There was a problem hiding this comment.
Code using assert but did not include the header cassert, rely on the unstable internal include relationship of STL. I think it would be better to fix it.
| @@ -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<std::pmr::wstring> _polyStrings; | |||
| std::pmr::vector<std::pmr::basic_string<int>> _polyWidths; | |||
| std::pmr::vector<std::pmr::vector<int>> _polyWidths; | |||
There was a problem hiding this comment.
Not used as string nor need the null terminator.
There was a problem hiding this comment.
I wonder why this line was changed with the title of this PR. IIRC, some developers use std::basic_string<T> for non-char types to express small arrays of type T in order to make use of SSO behaviour of std::basic_string<>. The null terminator is also only guaranteed to be present when .c_str() is invoked, if I'm not mistaken (please correct me otherwise :) )
There was a problem hiding this comment.
std::basic_string<int> is actually std::basic_string<int, std::char_traits<int>, std::allocator<int>>. However, the standard doesn't allow std::char_traits<int>. STL supports it because of historical mistake. When CharType is int32_t or char32_t, the STL (as well as other standard library implementations) have a very small SSO capacity of 3, making it essentially pointless. Since C++11, it is prohibited to add the null terminator when calling c_str.
YexuanXiao
changed the title
and fix non-standard compliant code
Fix some potential issues that may be exposed after upgrading the compiler and dependencies. For detailed explanations, see the comments.