diff --git a/src/cascadia/TerminalApp/Jumplist.cpp b/src/cascadia/TerminalApp/Jumplist.cpp index 6ca3483775d..d558c5433fc 100644 --- a/src/cascadia/TerminalApp/Jumplist.cpp +++ b/src/cascadia/TerminalApp/Jumplist.cpp @@ -162,10 +162,9 @@ winrt::com_ptr Jumplist::_createShellLink(const std::wstring_view n const std::wstring iconPath{ path.substr(0, commaPosition) }; // We dont want the comma included so add 1 to its position - int iconIndex = til::to_int(path.substr(commaPosition + 1)); - if (iconIndex != til::to_int_error) + if (const auto iconIndex = til::parse_signed(path.substr(commaPosition + 1))) { - THROW_IF_FAILED(sh->SetIconLocation(iconPath.data(), iconIndex)); + THROW_IF_FAILED(sh->SetIconLocation(iconPath.data(), *iconIndex)); } } else if (til::ends_with(path, L"exe") || til::ends_with(path, L"dll")) diff --git a/src/cascadia/TerminalSettingsModel/KeyChordSerialization.cpp b/src/cascadia/TerminalSettingsModel/KeyChordSerialization.cpp index 7be5aed057c..d77c02e42a8 100644 --- a/src/cascadia/TerminalSettingsModel/KeyChordSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/KeyChordSerialization.cpp @@ -101,10 +101,10 @@ static int32_t parseNumericCode(const std::wstring_view& str, const std::wstring return 0; } - const auto value = til::to_ulong({ str.data() + prefix.size(), str.size() - prefix.size() - suffix.size() }); - if (value > 0 && value < 256) + const auto value = til::parse_unsigned({ str.data() + prefix.size(), str.size() - prefix.size() - suffix.size() }); + if (value) { - return gsl::narrow_cast(value); + return gsl::narrow_cast(*value); } throw winrt::hresult_invalid_argument(L"Invalid numeric argument to vk() or sc()"); @@ -151,10 +151,8 @@ static KeyChord _fromString(std::wstring_view wstr) auto vkey = 0; auto scanCode = 0; - while (!wstr.empty()) + for (const auto& part : til::split_iterator{ wstr, L'+' }) { - const auto part = til::prefix_split(wstr, L'+'); - if (til::equals_insensitive_ascii(part, CTRL_KEY)) { modifiers |= VirtualKeyModifiers::Control; diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h b/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h index 4d2a84a5e6c..d497489c79d 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h +++ b/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h @@ -719,8 +719,8 @@ struct ::Microsoft::Terminal::Settings::Model::JsonUtils::ConversionTrait<::winr if (isIndexed16) { const auto indexStr = string.substr(1); - const auto idx = til::to_ulong(indexStr, 16); - color.r = gsl::narrow_cast(std::min(idx, 15ul)); + const auto idx = til::parse_unsigned(indexStr, 16); + color.r = std::min(idx.value_or(255), 15); } else { diff --git a/src/cascadia/UIHelpers/Converters.cpp b/src/cascadia/UIHelpers/Converters.cpp index 812d902102b..76d0c464b3f 100644 --- a/src/cascadia/UIHelpers/Converters.cpp +++ b/src/cascadia/UIHelpers/Converters.cpp @@ -73,35 +73,32 @@ namespace winrt::Microsoft::Terminal::UI::implementation double Converters::MaxValueFromPaddingString(const winrt::hstring& paddingString) { - std::wstring_view remaining{ paddingString }; + std::wstring buffer; double maxVal = 0; + auto& errnoRef = errno; // Nonzero cost, pay it once + // Get padding values till we run out of delimiter separated values in the stream // Non-numeral values detected will default to 0 // std::stod will throw invalid_argument exception if the input is an invalid double value // std::stod will throw out_of_range exception if the input value is more than DBL_MAX - try + for (const auto& part : til::split_iterator{ std::wstring_view{ paddingString }, L',' }) { - while (!remaining.empty()) + buffer.assign(part); + + // wcstod handles whitespace prefix (which is ignored) & stops the + // scan when first char outside the range of radix is encountered. + // We'll be permissive till the extent that stod function allows us to be by default + // Ex. a value like 100.3#535w2 will be read as 100.3, but ;df25 will fail + errnoRef = 0; + wchar_t* end; + const double val = wcstod(buffer.c_str(), &end); + + if (end != buffer.c_str() && errnoRef != ERANGE) { - const std::wstring token{ til::prefix_split(remaining, L',') }; - // std::stod internally calls wcstod which handles whitespace prefix (which is ignored) - // & stops the scan when first char outside the range of radix is encountered - // We'll be permissive till the extent that stod function allows us to be by default - // Ex. a value like 100.3#535w2 will be read as 100.3, but ;df25 will fail - const auto curVal = std::stod(token); - if (curVal > maxVal) - { - maxVal = curVal; - } + maxVal = std::max(maxVal, val); } } - catch (...) - { - // If something goes wrong, even if due to a single bad padding value, we'll return default 0 padding - maxVal = 0; - LOG_CAUGHT_EXCEPTION(); - } return maxVal; } diff --git a/src/cascadia/UIHelpers/IconPathConverter.cpp b/src/cascadia/UIHelpers/IconPathConverter.cpp index bb0e7e9352a..4d8bf2b90f7 100644 --- a/src/cascadia/UIHelpers/IconPathConverter.cpp +++ b/src/cascadia/UIHelpers/IconPathConverter.cpp @@ -271,12 +271,7 @@ namespace winrt::Microsoft::Terminal::UI::implementation if (commaIndex != std::wstring::npos) { // Convert the string iconIndex to a signed int to support negative numbers which represent an Icon's ID. - const auto index{ til::to_int(pathView.substr(commaIndex + 1)) }; - if (index == til::to_int_error) - { - return std::nullopt; - } - return static_cast(index); + return til::parse_signed(pathView.substr(commaIndex + 1)); } // We had a binary path, but no index. Default to 0. diff --git a/src/common.build.pre.props b/src/common.build.pre.props index 315902305ae..18b7e6a8b01 100644 --- a/src/common.build.pre.props +++ b/src/common.build.pre.props @@ -150,7 +150,17 @@ true stdcpp20 stdc17 - %(AdditionalOptions) /utf-8 /Zc:__cplusplus /Zc:__STDC__ /Zc:enumTypes /Zc:externConstexpr /Zc:templateScope /Zc:throwingNew + + %(AdditionalOptions) /utf-8 /Zc:__cplusplus /Zc:__STDC__ /Zc:enumTypes /Zc:inline /Zc:templateScope /Zc:throwingNew /diagnostics:caret Guard diff --git a/src/inc/til/string.h b/src/inc/til/string.h index 5070ae472fe..a3f16f75e62 100644 --- a/src/inc/til/string.h +++ b/src/inc/til/string.h @@ -133,113 +133,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return ends_with<>(str, prefix); } - inline constexpr unsigned long to_ulong_error = ULONG_MAX; - inline constexpr int to_int_error = INT_MAX; - - // Just like std::wcstoul, but without annoying locales and null-terminating strings. - // It has been fuzz-tested against clang's strtoul implementation. - template - _TIL_INLINEPREFIX constexpr unsigned long to_ulong(const std::basic_string_view& str, unsigned long base = 0) noexcept - { - static constexpr unsigned long maximumValue = ULONG_MAX / 16; - - // We don't have to test ptr for nullability, as we only access it under either condition: - // * str.length() > 0, for determining the base - // * ptr != end, when parsing the characters; if ptr is null, length will be 0 and thus end == ptr -#pragma warning(push) -#pragma warning(disable : 26429) // Symbol 'ptr' is never tested for nullness, it can be marked as not_null -#pragma warning(disable : 26481) // Don't use pointer arithmetic. Use span instead - auto ptr = str.data(); - const auto end = ptr + str.length(); - unsigned long accumulator = 0; - unsigned long value = ULONG_MAX; - - if (!base) - { - base = 10; - - if (str.length() > 1 && *ptr == '0') - { - base = 8; - ++ptr; - - if (str.length() > 2 && (*ptr == 'x' || *ptr == 'X')) - { - base = 16; - ++ptr; - } - } - } - - if (ptr == end) - { - return to_ulong_error; - } - - for (;; accumulator *= base) - { - value = ULONG_MAX; - if (*ptr >= '0' && *ptr <= '9') - { - value = *ptr - '0'; - } - else if (*ptr >= 'A' && *ptr <= 'F') - { - value = *ptr - 'A' + 10; - } - else if (*ptr >= 'a' && *ptr <= 'f') - { - value = *ptr - 'a' + 10; - } - else - { - return to_ulong_error; - } - - accumulator += value; - if (accumulator >= maximumValue) - { - return to_ulong_error; - } - - if (++ptr == end) - { - return accumulator; - } - } -#pragma warning(pop) - } - - constexpr unsigned long to_ulong(const std::string_view& str, unsigned long base = 0) noexcept - { - return to_ulong<>(str, base); - } - - constexpr unsigned long to_ulong(const std::wstring_view& str, unsigned long base = 0) noexcept - { - return to_ulong<>(str, base); - } - - // Implement to_int in terms of to_ulong by negating its result. to_ulong does not expect - // to be passed signed numbers and will return an error accordingly. That error when - // compared against -1 evaluates to true. We account for that by returning to_int_error if to_ulong - // returns an error. - constexpr int to_int(const std::wstring_view& str, unsigned long base = 0) noexcept - { - auto result = to_ulong_error; - const auto signPosition = str.find(L"-"); - const bool hasSign = signPosition != std::wstring_view::npos; - result = hasSign ? to_ulong(str.substr(signPosition + 1), base) : to_ulong(str, base); - - // Check that result is valid and will fit in an int. - if (result == to_ulong_error || (result > INT_MAX)) - { - return to_int_error; - } - - return hasSign ? result * -1 : result; - } - // Just like std::tolower, but without annoying locales. template constexpr T tolower_ascii(T c) @@ -348,70 +241,257 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return ends_with<>(str, prefix); } - // Give the arguments ("foo bar baz", " "), this method will - // * modify the first argument to "bar baz" - // * return "foo" - // If the needle cannot be found the "str" argument is returned as is. template - constexpr std::basic_string_view prefix_split(std::basic_string_view& str, const std::basic_string_view& needle) noexcept + constexpr std::basic_string_view trim(const std::basic_string_view& str, const T ch) noexcept { - using view_type = std::basic_string_view; - - const auto needleLen = needle.size(); - const auto idx = needleLen == 0 ? str.size() : str.find(needle); - const auto prefixIdx = std::min(str.size(), idx); - const auto suffixIdx = std::min(str.size(), prefixIdx + needle.size()); + auto beg = str.data(); + auto end = beg + str.size(); - const view_type result{ str.data(), prefixIdx }; -#pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead - str = { str.data() + suffixIdx, str.size() - suffixIdx }; - return result; - } + for (; beg != end && *beg == ch; ++beg) + { + } - constexpr std::string_view prefix_split(std::string_view& str, const std::string_view& needle) noexcept - { - return prefix_split<>(str, needle); - } + for (; beg != end && end[-1] == ch; --end) + { + } - constexpr std::wstring_view prefix_split(std::wstring_view& str, const std::wstring_view& needle) noexcept - { - return prefix_split<>(str, needle); + return { beg, end }; } - // Give the arguments ("foo bar baz", " "), this method will - // * modify the first argument to "bar baz" - // * return "foo" - // If the needle cannot be found the "str" argument is returned as is. + // The primary use-case for this is in for-range loops to split a string by a delimiter. + // For instance, to split a string by semicolon: + // for (const auto& token : wil::split_iterator{ str, L';' }) + // + // It's written in a way that lets MSVC optimize away the _advance flag and + // the ternary in advance(). The resulting assembly is quite alright. template - constexpr std::basic_string_view prefix_split(std::basic_string_view& str, T ch) noexcept + struct split_iterator { - using view_type = std::basic_string_view; + struct sentinel + { + }; - const auto idx = str.find(ch); - const auto prefixIdx = std::min(str.size(), idx); - const auto suffixIdx = std::min(str.size(), prefixIdx + 1); + struct iterator + { + using iterator_category = std::forward_iterator_tag; + using value_type = std::basic_string_view; + using reference = value_type&; + using pointer = value_type*; + using difference_type = std::ptrdiff_t; + + explicit constexpr iterator(split_iterator& p) noexcept + : + _iter{ p } + { + } - const view_type result{ str.data(), prefixIdx }; -#pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead - str = { str.data() + suffixIdx, str.size() - suffixIdx }; - return result; - } + const value_type& operator*() const noexcept + { + return _iter.value(); + } - template - constexpr std::basic_string_view trim(const std::basic_string_view& str, const T ch) noexcept + iterator& operator++() noexcept + { + _iter.advance(); + return *this; + } + + bool operator!=(const sentinel&) const noexcept + { + return _iter.valid(); + } + + private: + split_iterator& _iter; + }; + + explicit constexpr split_iterator(const std::basic_string_view& str, T needle) noexcept : + _it{ str.begin() }, + _tok{ str.begin() }, + _end{ str.end() }, + _needle{ needle } + { + } + + iterator begin() noexcept + { + return iterator{ *this }; + } + + sentinel end() noexcept + { + return sentinel{}; + } + + private: + bool valid() const noexcept + { + return _tok != _end; + } + + void advance() noexcept + { + _it = _tok == _end ? _end : _tok + 1; + _advance = true; + } + + const typename iterator::value_type& value() noexcept + { + if (_advance) + { + _tok = std::find(_it, _end, _needle); + _value = { _it, _tok }; + _advance = false; + } + return _value; + } + + typename iterator::value_type::iterator _it; + typename iterator::value_type::iterator _tok; + typename iterator::value_type::iterator _end; + typename iterator::value_type _value; + T _needle; + bool _advance = true; + }; + + namespace details { - auto beg = str.data(); - auto end = beg + str.size(); + // Just like std::wcstoul, but without annoying locales and null-terminating strings. + template + _TIL_INLINEPREFIX constexpr std::optional parse_u64(const std::basic_string_view& str, int base = 0) noexcept + { + // We don't have to test ptr for nullability, as we only access it under either condition: + // * str.length() > 0, for determining the base + // * ptr != end, when parsing the characters; if ptr is null, length will be 0 and thus end == ptr +#pragma warning(push) +#pragma warning(disable : 26429) // Symbol 'ptr' is never tested for nullness, it can be marked as not_null +#pragma warning(disable : 26451) // Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. [...] +#pragma warning(disable : 26481) // Don't use pointer arithmetic. Use span instead + auto ptr = str.data(); + const auto end = ptr + str.length(); + uint64_t accumulator = 0; + uint64_t base64 = base; - for (; beg != end && *beg == ch; ++beg) + if (base <= 0) + { + base64 = 10; + + if (ptr != end && *ptr == '0') + { + base64 = 8; + ptr += 1; + + if (ptr != end && (*ptr == 'x' || *ptr == 'X')) + { + base64 = 16; + ptr += 1; + } + } + } + + if (ptr == end) + { + return {}; + } + + for (;;) + { + uint64_t value = 0; + if (*ptr >= '0' && *ptr <= '9') + { + value = *ptr - '0'; + } + else if (*ptr >= 'A' && *ptr <= 'F') + { + value = *ptr - 'A' + 10; + } + else if (*ptr >= 'a' && *ptr <= 'f') + { + value = *ptr - 'a' + 10; + } + else + { + return {}; + } + + const auto acc = accumulator * base64 + value; + if (acc < accumulator) + { + return {}; + } + + accumulator = acc; + ptr += 1; + + if (ptr == end) + { + return accumulator; + } + } +#pragma warning(pop) + } + + template + constexpr std::optional parse_unsigned(const std::basic_string_view& str, int base = 0) noexcept { + if constexpr (std::is_same_v) + { + return details::parse_u64<>(str, base); + } + else + { + const auto opt = details::parse_u64<>(str, base); + if (!opt || *opt > uint64_t{ std::numeric_limits::max() }) + { + return {}; + } + return gsl::narrow_cast(*opt); + } } - for (; beg != end && end[-1] == ch; --end) + template + constexpr std::optional parse_signed(std::basic_string_view str, int base = 0) noexcept { + const bool hasSign = str.starts_with(L'-'); + if (hasSign) + { + str = str.substr(1); + } + + const auto opt = details::parse_u64<>(str, base); + const auto max = gsl::narrow_cast(std::numeric_limits::max()) + hasSign; + if (!opt || *opt > max) + { + return {}; + } + + const auto r = gsl::narrow_cast(*opt); + return hasSign ? -r : r; } + } - return { beg, end }; + template + constexpr std::optional parse_unsigned(const std::string_view& str, int base = 0) noexcept + { + return details::parse_unsigned(str, base); + } + + template + constexpr std::optional parse_unsigned(const std::wstring_view& str, int base = 0) noexcept + { + return details::parse_unsigned(str, base); + } + + template + constexpr std::optional parse_signed(const std::string_view& str, int base = 0) noexcept + { + return details::parse_signed(str, base); + } + + template + constexpr std::optional parse_signed(const std::wstring_view& str, int base = 0) noexcept + { + return details::parse_signed(str, base); } // Splits a font-family list into individual font-families. It loosely follows the CSS spec for font-family. diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp index edb53e100d0..7f9563e3635 100644 --- a/src/terminal/parser/OutputStateMachineEngine.cpp +++ b/src/terminal/parser/OutputStateMachineEngine.cpp @@ -1015,7 +1015,7 @@ bool OutputStateMachineEngine::_GetOscSetColor(const std::wstring_view string, } std::vector newRgbs; - for (auto&& part : parts) + for (const auto& part : parts) { if (part == L"?"sv) [[unlikely]] { diff --git a/src/til/ut_til/string.cpp b/src/til/ut_til/string.cpp index 2ef656a04d7..c3cf9a9dd03 100644 --- a/src/til/ut_til/string.cpp +++ b/src/til/ut_til/string.cpp @@ -72,9 +72,9 @@ class StringTests VERIFY_IS_TRUE(til::ends_with("0abc", "abc")); } - // Normally this would be the spot where you'd find a TEST_METHOD(to_ulong). + // Normally this would be the spot where you'd find a TEST_METHOD(parse_u64). // I didn't quite trust my coding skills and thus opted to use fuzz-testing. - // The below function was used to test to_ulong for unsafety and conformance with clang's strtoul. + // The below function was used to test parse_u64 for unsafety and conformance with clang's strtoul. // The test was run as: // clang++ -fsanitize=address,undefined,fuzzer -std=c++17 file.cpp // and was run for 20min across 16 jobs in parallel. @@ -112,7 +112,7 @@ class StringTests return 0; } - const auto actual = to_ulong({ wide_buffer, size }); + const auto actual = parse_u64({ wide_buffer, size }); if (expected != actual) { __builtin_trap(); @@ -147,64 +147,39 @@ class StringTests VERIFY_IS_TRUE(til::equals_insensitive_ascii("cOUnterStriKE", "COuntERStRike")); } - TEST_METHOD(prefix_split) + TEST_METHOD(split_iterator) { - { - std::string_view s{ "" }; - VERIFY_ARE_EQUAL("", til::prefix_split(s, "")); - VERIFY_ARE_EQUAL("", s); - } - { - std::string_view s{ "" }; - VERIFY_ARE_EQUAL("", til::prefix_split(s, " ")); - VERIFY_ARE_EQUAL("", s); - } - { - std::string_view s{ " " }; - VERIFY_ARE_EQUAL(" ", til::prefix_split(s, "")); - VERIFY_ARE_EQUAL("", s); - } - { - std::string_view s{ "foo" }; - VERIFY_ARE_EQUAL("foo", til::prefix_split(s, "")); - VERIFY_ARE_EQUAL("", s); - } - { - std::string_view s{ "foo bar baz" }; - VERIFY_ARE_EQUAL("foo", til::prefix_split(s, " ")); - VERIFY_ARE_EQUAL("bar baz", s); - VERIFY_ARE_EQUAL("bar", til::prefix_split(s, " ")); - VERIFY_ARE_EQUAL("baz", s); - VERIFY_ARE_EQUAL("baz", til::prefix_split(s, " ")); - VERIFY_ARE_EQUAL("", s); - } - { - std::string_view s{ "foo123barbaz123" }; - VERIFY_ARE_EQUAL("foo", til::prefix_split(s, "123")); - VERIFY_ARE_EQUAL("barbaz123", s); - VERIFY_ARE_EQUAL("barbaz", til::prefix_split(s, "123")); - VERIFY_ARE_EQUAL("", s); - VERIFY_ARE_EQUAL("", til::prefix_split(s, "")); - VERIFY_ARE_EQUAL("", s); - } - } + static constexpr auto split = [](const std::string_view& str, const char needle) { + std::vector substrings; + for (auto&& s : til::split_iterator{ str, needle }) + { + substrings.emplace_back(s); + } + return substrings; + }; - TEST_METHOD(prefix_split_char) - { - { - std::string_view s{ "" }; - VERIFY_ARE_EQUAL("", til::prefix_split(s, ' ')); - VERIFY_ARE_EQUAL("", s); - } - { - std::string_view s{ "foo bar baz" }; - VERIFY_ARE_EQUAL("foo", til::prefix_split(s, ' ')); - VERIFY_ARE_EQUAL("bar baz", s); - VERIFY_ARE_EQUAL("bar", til::prefix_split(s, ' ')); - VERIFY_ARE_EQUAL("baz", s); - VERIFY_ARE_EQUAL("baz", til::prefix_split(s, ' ')); - VERIFY_ARE_EQUAL("", s); - } + std::vector expected; + std::vector actual; + + expected = { "foo" }; + actual = split("foo", ' '); + VERIFY_ARE_EQUAL(expected, actual); + + expected = { "", "foo" }; + actual = split(" foo", ' '); + VERIFY_ARE_EQUAL(expected, actual); + + expected = { "foo", "" }; + actual = split("foo ", ' '); + VERIFY_ARE_EQUAL(expected, actual); + + expected = { "foo", "bar", "baz" }; + actual = split("foo bar baz", ' '); + VERIFY_ARE_EQUAL(expected, actual); + + expected = { "", "", "foo", "", "bar", "", "" }; + actual = split(";;foo;;bar;;", ';'); + VERIFY_ARE_EQUAL(expected, actual); } TEST_METHOD(CleanPathAndFilename)