Move to GSL 3.1.0 (#6908)

GSL 3, the next major version of GSL after the one we're using, replaced
their local implementation of `span` with one that more closely mimics
C++20's span. Unfortunately, that is a breaking change for all of GSL's
consumers.

This commit updates our use of span to comply with the new changes in
GSL 3.

Chief among those breaking changes is:

* `span::at` no longer exists; I replaced many instances of `span::at`
  with `gsl::at(x)`
* `span::size_type` has finally given up on `ptrdiff_t` and become
  `size_t` like all other containers

While I was here, I also made the following mechanical replacements:

* In some of our "early standardized" code, we used std::optional's
  `has_value` and `value` back-to-back. Each `value` incurs an
  additional presence test.
  * Change: `x.value().member` -> `x->member` (`optional::operator->`
    skips the presence test)
  * Change: `x.value()` -> `*x` (as above)
* GSL 3 uses `size_t` for `size_type`.
  * Change: `gsl::narrow<size_t>(x.size())` -> `x.size()`
  * Change: `gsl::narrow<ptrdiff_t>(nonSpan.size())` -> `nonSpan.size()`
    during span construction

I also replaced two instances of `x[x.size() - 1]` with `x.back()` and
one instance of a manual array walk (for comparison) with a direct
comparison.

NOTE: Span comparison and `make_span` are not part of the C++20 span
library.

Fixes #6251

Author: DHowett

dep/gsl

@@ -927,7 +927,7 @@ void Terminal::SetBackgroundCallback(std::function<void(const COLORREF)> pfn) no
 void Terminal::_InitializeColorTable()
 try
 {
-    const gsl::span<COLORREF> tableView = { _colorTable.data(), gsl::narrow<ptrdiff_t>(_colorTable.size()) };
+    const gsl::span<COLORREF> tableView = { _colorTable.data(), _colorTable.size() };
     // First set up the basic 256 colors
     Utils::Initialize256ColorTable(tableView);
     // Then use fill the first 16 values with the Campbell scheme

src/cascadia/TerminalCore/Terminal.cpp

@@ -102,7 +102,7 @@ namespace TerminalAppUnitTests
         VERIFY_ARE_EQUAL(ARGB(0, 0xFF, 0xFF, 0xFF), scheme.GetCursorColor());
 
         std::array<COLORREF, COLOR_TABLE_SIZE> expectedCampbellTable;
-        auto campbellSpan = gsl::span<COLORREF>(&expectedCampbellTable[0], gsl::narrow<ptrdiff_t>(COLOR_TABLE_SIZE));
+        auto campbellSpan = gsl::span<COLORREF>(&expectedCampbellTable[0], COLOR_TABLE_SIZE);
         Utils::InitializeCampbellColorTable(campbellSpan);
         Utils::SetColorTableAlpha(campbellSpan, 0);
 

src/cascadia/ut_app/JsonTests.cpp

@@ -147,9 +147,9 @@ std::unordered_map<std::wstring,
     // Ensure output variables are initialized
     writtenOrNeeded = 0;
 
-    if (target.has_value() && target.value().size() > 0)
+    if (target.has_value() && target->size() > 0)
     {
-        target.value().at(0) = UNICODE_NULL;
+        gsl::at(*target, 0) = UNICODE_NULL;
     }
 
     std::wstring exeNameString(exeName);
@@ -178,9 +178,9 @@ std::unordered_map<std::wstring,
     if (target.has_value())
     {
         // if the user didn't give us enough space, return with insufficient buffer code early.
-        RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_INSUFFICIENT_BUFFER), gsl::narrow<size_t>(target.value().size()) < neededSize);
+        RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_INSUFFICIENT_BUFFER), target->size() < neededSize);
 
-        RETURN_IF_FAILED(StringCchCopyNW(target.value().data(), target.value().size(), targetString.data(), targetSize));
+        RETURN_IF_FAILED(StringCchCopyNW(target->data(), target->size(), targetString.data(), targetSize));
     }
 
     return S_OK;
@@ -211,7 +211,7 @@ std::unordered_map<std::wstring,
     {
         if (target.size() > 0)
         {
-            target.at(0) = ANSI_NULL;
+            gsl::at(target, 0) = ANSI_NULL;
         }
 
         LockConsole();
@@ -449,14 +449,14 @@ void Alias::s_ClearCmdExeAliases()
     // Ensure output variables are initialized.
     writtenOrNeeded = 0;
 
-    if (aliasBuffer.has_value() && aliasBuffer.value().size() > 0)
+    if (aliasBuffer.has_value() && aliasBuffer->size() > 0)
     {
-        aliasBuffer.value().at(0) = UNICODE_NULL;
+        gsl::at(*aliasBuffer, 0) = UNICODE_NULL;
     }
 
     std::wstring exeNameString(exeName);
 
-    LPWSTR AliasesBufferPtrW = aliasBuffer.has_value() ? aliasBuffer.value().data() : nullptr;
+    LPWSTR AliasesBufferPtrW = aliasBuffer.has_value() ? aliasBuffer->data() : nullptr;
     size_t cchTotalLength = 0; // accumulate the characters we need/have copied as we walk the list
 
     // Each of the aliases will be made up of the source, a separator, the target, then a null character.
@@ -489,10 +489,10 @@ void Alias::s_ClearCmdExeAliases()
                 size_t cchNewTotal;
                 RETURN_IF_FAILED(SizeTAdd(cchTotalLength, cchNeeded, &cchNewTotal));
 
-                RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW), cchNewTotal > gsl::narrow<size_t>(aliasBuffer.value().size()));
+                RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW), cchNewTotal > aliasBuffer->size());
 
                 size_t cchAliasBufferRemaining;
-                RETURN_IF_FAILED(SizeTSub(aliasBuffer.value().size(), cchTotalLength, &cchAliasBufferRemaining));
+                RETURN_IF_FAILED(SizeTSub(aliasBuffer->size(), cchTotalLength, &cchAliasBufferRemaining));
 
                 RETURN_IF_FAILED(StringCchCopyNW(AliasesBufferPtrW, cchAliasBufferRemaining, pair.first.data(), cchSource));
                 RETURN_IF_FAILED(SizeTSub(cchAliasBufferRemaining, cchSource, &cchAliasBufferRemaining));
@@ -543,7 +543,7 @@ void Alias::s_ClearCmdExeAliases()
     {
         if (alias.size() > 0)
         {
-            alias.at(0) = '\0';
+            gsl::at(alias, 0) = '\0';
         }
 
         LockConsole();
@@ -574,7 +574,7 @@ void Alias::s_ClearCmdExeAliases()
         // Copy safely to the output buffer
         // - Aliases are a series of null terminated strings. We cannot use a SafeString function to copy.
         //   So instead, validate and use raw memory copy.
-        RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW), converted.size() > gsl::narrow<size_t>(alias.size()));
+        RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW), converted.size() > alias.size());
         memcpy_s(alias.data(), alias.size(), converted.data(), converted.size());
 
         // And return the size copied.
@@ -696,12 +696,12 @@ void Alias::s_ClearCmdExeAliases()
 {
     // Ensure output variables are initialized.
     writtenOrNeeded = 0;
-    if (aliasExesBuffer.has_value() && aliasExesBuffer.value().size() > 0)
+    if (aliasExesBuffer.has_value() && aliasExesBuffer->size() > 0)
     {
-        aliasExesBuffer.value().at(0) = UNICODE_NULL;
+        gsl::at(*aliasExesBuffer, 0) = UNICODE_NULL;
     }
 
-    LPWSTR AliasExesBufferPtrW = aliasExesBuffer.has_value() ? aliasExesBuffer.value().data() : nullptr;
+    LPWSTR AliasExesBufferPtrW = aliasExesBuffer.has_value() ? aliasExesBuffer->data() : nullptr;
     size_t cchTotalLength = 0; // accumulate the characters we need/have copied as we walk the list
 
     size_t const cchNull = 1;
@@ -722,10 +722,10 @@ void Alias::s_ClearCmdExeAliases()
             // Error out early if there is a problem.
             size_t cchNewTotal;
             RETURN_IF_FAILED(SizeTAdd(cchTotalLength, cchNeeded, &cchNewTotal));
-            RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW), cchNewTotal > gsl::narrow<size_t>(aliasExesBuffer.value().size()));
+            RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW), cchNewTotal > aliasExesBuffer->size());
 
             size_t cchRemaining;
-            RETURN_IF_FAILED(SizeTSub(aliasExesBuffer.value().size(), cchTotalLength, &cchRemaining));
+            RETURN_IF_FAILED(SizeTSub(aliasExesBuffer->size(), cchTotalLength, &cchRemaining));
 
             RETURN_IF_FAILED(StringCchCopyNW(AliasExesBufferPtrW, cchRemaining, pair.first.data(), cchExe));
             AliasExesBufferPtrW += cchNeeded;
@@ -761,7 +761,7 @@ void Alias::s_ClearCmdExeAliases()
     {
         if (aliasExes.size() > 0)
         {
-            aliasExes.at(0) = '\0';
+            gsl::at(aliasExes, 0) = '\0';
         }
 
         LockConsole();
@@ -788,7 +788,7 @@ void Alias::s_ClearCmdExeAliases()
         // Copy safely to the output buffer
         // - AliasExes are a series of null terminated strings. We cannot use a SafeString function to copy.
         //   So instead, validate and use raw memory copy.
-        RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW), converted.size() > gsl::narrow<size_t>(aliasExes.size()));
+        RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW), converted.size() > aliasExes.size());
         memcpy_s(aliasExes.data(), aliasExes.size(), converted.data(), converted.size());
 
         // And return the size copied.

src/host/alias.cpp

@@ -64,13 +64,15 @@ DWORD UnicodeRasterFontCellMungeOnRead(const gsl::span<CHAR_INFO> buffer)
     DWORD iDst = 0;
 
     // Walk through every CHAR_INFO
-    for (DWORD iSrc = 0; iSrc < gsl::narrow<size_t>(buffer.size()); iSrc++)
+    for (DWORD iSrc = 0; iSrc < buffer.size(); iSrc++)
     {
         // If it's not a trailing byte, copy it straight over, stripping out the Leading/Trailing flags from the attributes field.
-        if (!WI_IsFlagSet(buffer.at(iSrc).Attributes, COMMON_LVB_TRAILING_BYTE))
+        auto& src{ gsl::at(buffer, iSrc) };
+        if (!WI_IsFlagSet(src.Attributes, COMMON_LVB_TRAILING_BYTE))
         {
-            buffer.at(iDst) = buffer.at(iSrc);
-            WI_ClearAllFlags(buffer.at(iDst).Attributes, COMMON_LVB_SBCSDBCS);
+            auto& dst{ gsl::at(buffer, iDst) };
+            dst = src;
+            WI_ClearAllFlags(dst.Attributes, COMMON_LVB_SBCSDBCS);
             iDst++;
         }
 
@@ -90,7 +92,7 @@ DWORD UnicodeRasterFontCellMungeOnRead(const gsl::span<CHAR_INFO> buffer)
     iDst += cchDstToClear;
 
     // now that we're done, we should have copied, left alone, or cleared the entire length.
-    FAIL_FAST_IF(!(iDst == gsl::narrow<size_t>(buffer.size())));
+    FAIL_FAST_IF(iDst != buffer.size());
 
     return iDst;
 }

src/host/dbcs.cpp

@@ -610,7 +610,7 @@ void EventsToUnicode(_Inout_ std::deque<std::unique_ptr<IInputEvent>>& inEvents,
 {
     try
     {
-        std::vector<CHAR_INFO> tempBuffer(buffer.cbegin(), buffer.cend());
+        std::vector<CHAR_INFO> tempBuffer(buffer.begin(), buffer.end());
 
         const auto size = rectangle.Dimensions();
         auto tempIter = tempBuffer.cbegin();
@@ -761,7 +761,7 @@ void EventsToUnicode(_Inout_ std::deque<std::unique_ptr<IInputEvent>>& inEvents,
     result.reserve(buffer.size() * 2); // we estimate we'll need up to double the cells if they all expand.
 
     const auto size = rectangle.Dimensions();
-    auto bufferIter = buffer.cbegin();
+    auto bufferIter = buffer.begin();
 
     for (SHORT i = 0; i < size.Y; i++)
     {
@@ -835,9 +835,8 @@ void EventsToUnicode(_Inout_ std::deque<std::unique_ptr<IInputEvent>>& inEvents,
         }
 
         // The buffer given should be big enough to hold the dimensions of the request.
-        ptrdiff_t targetArea;
-        RETURN_IF_FAILED(PtrdiffTMult(targetSize.X, targetSize.Y, &targetArea));
-        RETURN_HR_IF(E_INVALIDARG, targetArea < 0);
+        size_t targetArea;
+        RETURN_IF_FAILED(SizeTMult(targetSize.X, targetSize.Y, &targetArea));
         RETURN_HR_IF(E_INVALIDARG, targetArea < targetBuffer.size());
 
         // Clip the request rectangle to the size of the storage buffer
@@ -1145,7 +1144,7 @@ void EventsToUnicode(_Inout_ std::deque<std::unique_ptr<IInputEvent>>& inEvents,
 
         // for compatibility reasons, if we receive more chars than can fit in the buffer
         // then we don't send anything back.
-        if (chars.size() <= gsl::narrow<size_t>(buffer.size()))
+        if (chars.size() <= buffer.size())
         {
             std::copy(chars.cbegin(), chars.cend(), buffer.begin());
             written = chars.size();
@@ -1173,7 +1172,7 @@ void EventsToUnicode(_Inout_ std::deque<std::unique_ptr<IInputEvent>>& inEvents,
                                              buffer.size());
 
         // Only copy if the whole result will fit.
-        if (chars.size() <= gsl::narrow<size_t>(buffer.size()))
+        if (chars.size() <= buffer.size())
         {
             std::copy(chars.cbegin(), chars.cend(), buffer.begin());
             written = chars.size();

src/host/directio.cpp

@@ -1602,9 +1602,9 @@ void DoSrvPrivateRefreshWindow(_In_ const SCREEN_INFORMATION& screenInfo)
         written = 0;
         needed = 0;
 
-        if (title.has_value() && title.value().size() > 0)
+        if (title.has_value() && title->size() > 0)
         {
-            title.value().at(0) = ANSI_NULL;
+            gsl::at(*title, 0) = ANSI_NULL;
         }
 
         // Get the appropriate title and length depending on the mode.
@@ -1628,13 +1628,13 @@ void DoSrvPrivateRefreshWindow(_In_ const SCREEN_INFORMATION& screenInfo)
         // If we have a pointer to receive the data, then copy it out.
         if (title.has_value())
         {
-            HRESULT const hr = StringCchCopyNW(title.value().data(), title.value().size(), pwszTitle, cchTitleLength);
+            HRESULT const hr = StringCchCopyNW(title->data(), title->size(), pwszTitle, cchTitleLength);
 
             // Insufficient buffer is allowed. If we return a partial string, that's still OK by historical/compat standards.
             // Just say how much we managed to return.
             if (SUCCEEDED(hr) || STRSAFE_E_INSUFFICIENT_BUFFER == hr)
             {
-                written = std::min(gsl::narrow<size_t>(title.value().size()), cchTitleLength);
+                written = std::min(title->size(), cchTitleLength);
             }
         }
         return S_OK;
@@ -1667,7 +1667,7 @@ void DoSrvPrivateRefreshWindow(_In_ const SCREEN_INFORMATION& screenInfo)
 
         if (title.size() > 0)
         {
-            title.at(0) = ANSI_NULL;
+            gsl::at(title, 0) = ANSI_NULL;
         }
 
         // Figure out how big our temporary Unicode buffer must be to get the title.
@@ -1694,7 +1694,7 @@ void DoSrvPrivateRefreshWindow(_In_ const SCREEN_INFORMATION& screenInfo)
         // The legacy A behavior is a bit strange. If the buffer given doesn't have enough space to hold
         // the string without null termination (e.g. the title is 9 long, 10 with null. The buffer given isn't >= 9).
         // then do not copy anything back and do not report how much space we need.
-        if (gsl::narrow<size_t>(title.size()) >= converted.size())
+        if (title.size() >= converted.size())
         {
             // Say how many characters of buffer we would need to hold the entire result.
             needed = converted.size();
@@ -1707,13 +1707,13 @@ void DoSrvPrivateRefreshWindow(_In_ const SCREEN_INFORMATION& screenInfo)
             if (SUCCEEDED(hr) || STRSAFE_E_INSUFFICIENT_BUFFER == hr)
             {
                 // And return the size copied (either the size of the buffer or the null terminated length of the string we filled it with.)
-                written = std::min(gsl::narrow<size_t>(title.size()), converted.size() + 1);
+                written = std::min(title.size(), converted.size() + 1);
 
                 // Another compatibility fix... If we had exactly the number of bytes needed for an unterminated string,
                 // then replace the terminator left behind by StringCchCopyNA with the final character of the title string.
-                if (gsl::narrow<size_t>(title.size()) == converted.size())
+                if (title.size() == converted.size())
                 {
-                    title.at(title.size() - 1) = converted.data()[converted.size() - 1];
+                    title.back() = converted.back();
                 }
             }
         }
@@ -1722,7 +1722,7 @@ void DoSrvPrivateRefreshWindow(_In_ const SCREEN_INFORMATION& screenInfo)
             // If we didn't copy anything back and there is space, null terminate the given buffer and return.
             if (title.size() > 0)
             {
-                title.at(0) = ANSI_NULL;
+                gsl::at(title, 0) = ANSI_NULL;
                 written = 1;
             }
         }

src/host/getset.cpp

@@ -782,7 +782,7 @@ HRESULT GetConsoleCommandHistoryWImplHelper(const std::wstring_view exeName,
     writtenOrNeeded = 0;
     if (historyBuffer.size() > 0)
     {
-        historyBuffer.at(0) = UNICODE_NULL;
+        gsl::at(historyBuffer, 0) = UNICODE_NULL;
     }
 
     CommandHistory* const CommandHistory = CommandHistory::s_FindByExe(exeName);
@@ -812,7 +812,7 @@ HRESULT GetConsoleCommandHistoryWImplHelper(const std::wstring_view exeName,
                 size_t cchNewTotal;
                 RETURN_IF_FAILED(SizeTAdd(cchTotalLength, cchNeeded, &cchNewTotal));
 
-                RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW), cchNewTotal > gsl::narrow<size_t>(historyBuffer.size()));
+                RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW), cchNewTotal > historyBuffer.size());
 
                 size_t cchRemaining;
                 RETURN_IF_FAILED(SizeTSub(historyBuffer.size(),
@@ -859,7 +859,7 @@ HRESULT ApiRoutines::GetConsoleCommandHistoryAImpl(const std::string_view exeNam
     {
         if (commandHistory.size() > 0)
         {
-            commandHistory.at(0) = ANSI_NULL;
+            gsl::at(commandHistory, 0) = ANSI_NULL;
         }
 
         LockConsole();
@@ -889,7 +889,7 @@ HRESULT ApiRoutines::GetConsoleCommandHistoryAImpl(const std::string_view exeNam
         // Copy safely to output buffer
         // - CommandHistory are a series of null terminated strings. We cannot use a SafeString function to copy.
         //   So instead, validate and use raw memory copy.
-        RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW), converted.size() > gsl::narrow<size_t>(commandHistory.size()));
+        RETURN_HR_IF(HRESULT_FROM_WIN32(ERROR_BUFFER_OVERFLOW), converted.size() > commandHistory.size());
         memcpy_s(commandHistory.data(), commandHistory.size(), converted.data(), converted.size());
 
         // And return the size copied.

src/host/history.cpp

@@ -81,7 +81,7 @@ Settings::Settings() :
     _CursorColor = Cursor::s_InvertCursorColor;
     _CursorType = CursorType::Legacy;
 
-    gsl::span<COLORREF> tableView = { _colorTable.data(), gsl::narrow<ptrdiff_t>(_colorTable.size()) };
+    gsl::span<COLORREF> tableView = { _colorTable.data(), _colorTable.size() };
     ::Microsoft::Console::Utils::Initialize256ColorTable(tableView);
     ::Microsoft::Console::Utils::InitializeCampbellColorTableForConhost(tableView);
 }
@@ -122,7 +122,7 @@ void Settings::ApplyDesktopSpecificDefaults()
     _uNumberOfHistoryBuffers = 4;
     _bHistoryNoDup = FALSE;
 
-    gsl::span<COLORREF> tableView = { _colorTable.data(), gsl::narrow<ptrdiff_t>(_colorTable.size()) };
+    gsl::span<COLORREF> tableView = { _colorTable.data(), _colorTable.size() };
     ::Microsoft::Console::Utils::InitializeCampbellColorTableForConhost(tableView);
 
     _fTrimLeadingZeros = false;

src/host/settings.cpp

@@ -67,10 +67,7 @@ class CommandLineTests
     {
         const auto span = cookedReadData.SpanWholeBuffer();
         VERIFY_ARE_EQUAL(cookedReadData._bytesRead, wstr.size() * sizeof(wchar_t));
-        for (size_t i = 0; i < wstr.size(); ++i)
-        {
-            VERIFY_ARE_EQUAL(span.at(i), wstr.at(i));
-        }
+        VERIFY_ARE_EQUAL(wstr, (std::wstring_view{ span.data(), cookedReadData._bytesRead / sizeof(wchar_t) }));
     }
 
     void InitCookedReadData(COOKED_READ_DATA& cookedReadData,

src/host/ut_host/CommandLineTests.cpp

@@ -64,6 +64,7 @@
 #ifndef BLOCK_GSL
 #define GSL_MULTI_SPAN_H
 #include <gsl/gsl>
+#include <gsl/span_ext>
 #endif
 
 // CppCoreCheck

src/inc/LibraryIncludes.h

@@ -446,7 +446,7 @@ CATCH_RETURN()
     // Draw the background
     // The rectangle needs to be deduced based on the origin and the BidiDirection
     const auto advancesSpan = gsl::make_span(glyphRun->glyphAdvances, glyphRun->glyphCount);
-    const auto totalSpan = std::accumulate(advancesSpan.cbegin(), advancesSpan.cend(), 0.0f);
+    const auto totalSpan = std::accumulate(advancesSpan.begin(), advancesSpan.end(), 0.0f);
 
     D2D1_RECT_F rect;
     rect.top = origin.y;

src/renderer/dx/CustomTextRenderer.cpp

@@ -52,8 +52,8 @@ void UtilsTests::TestSwapColorPalette()
     std::array<COLORREF, COLOR_TABLE_SIZE> terminalTable;
     std::array<COLORREF, COLOR_TABLE_SIZE> consoleTable;
 
-    gsl::span<COLORREF> terminalTableView = { &terminalTable[0], gsl::narrow<ptrdiff_t>(terminalTable.size()) };
-    gsl::span<COLORREF> consoleTableView = { &consoleTable[0], gsl::narrow<ptrdiff_t>(consoleTable.size()) };
+    gsl::span<COLORREF> terminalTableView = { &terminalTable[0], terminalTable.size() };
+    gsl::span<COLORREF> consoleTableView = { &consoleTable[0], consoleTable.size() };
 
     // First set up the colors
     InitializeCampbellColorTable(terminalTableView);