From cecf553a85aba6657ec29d1b3ede3ecfd1c717ef Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 9 Mar 2020 12:10:53 -0700 Subject: [PATCH 1/9] Submit initial proposal for til::size including tests. --- src/inc/LibraryIncludes.h | 2 + src/inc/til.h | 1 + src/inc/til/size.h | 261 ++++++++++ src/til/precomp.h | 10 + src/til/ut_til/SizeTests.cpp | 486 ++++++++++++++++++ src/til/ut_til/SomeTests.cpp | 1 - src/til/ut_til/til.unit.tests.vcxproj | 1 + src/til/ut_til/til.unit.tests.vcxproj.filters | 15 + 8 files changed, 776 insertions(+), 1 deletion(-) create mode 100644 src/inc/til/size.h create mode 100644 src/til/ut_til/SizeTests.cpp create mode 100644 src/til/ut_til/til.unit.tests.vcxproj.filters diff --git a/src/inc/LibraryIncludes.h b/src/inc/LibraryIncludes.h index 61399cde05d..2002fa48fb0 100644 --- a/src/inc/LibraryIncludes.h +++ b/src/inc/LibraryIncludes.h @@ -86,7 +86,9 @@ #include // TIL - Terminal Implementation Library +#ifndef BLOCK_TIL // Certain projects may want to include TIL manually to gain superpowers #include "til.h" +#endif #pragma warning(pop) diff --git a/src/inc/til.h b/src/inc/til.h index abd12c59e9c..c590bf4e80d 100644 --- a/src/inc/til.h +++ b/src/inc/til.h @@ -5,6 +5,7 @@ #include "til/at.h" #include "til/some.h" +#include "til/size.h" #include "til/u8u16convert.h" namespace til // Terminal Implementation Library. Also: "Today I Learned" diff --git a/src/inc/til/size.h b/src/inc/til/size.h new file mode 100644 index 00000000000..3940184485b --- /dev/null +++ b/src/inc/til/size.h @@ -0,0 +1,261 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +#ifdef UNIT_TESTING +class SizeTests; +#endif + +namespace til // Terminal Implementation Library. Also: "Today I Learned" +{ + class size + { + public: + constexpr size() noexcept : + size(0, 0) + { + + } + +#if defined(_M_AMD64) || defined(_M_ARM64) + constexpr size(int width, int height) noexcept : + size(static_cast(width), static_cast(height)) + { + + } +#endif + + size(size_t width, size_t height) + { + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(width).AssignIfValid(&_width)); + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(height).AssignIfValid(&_height)); + } + + constexpr size(ptrdiff_t width, ptrdiff_t height) noexcept : + _width(width), + _height(height) + { + + } + + template + constexpr size(const TOther& other, std::enable_if_t().X)> && std::is_integral_v().Y)>, int> /*sentinel*/ = 0) : + size(static_cast(other.X), static_cast(other.Y)) + { + + } + + template + constexpr size(const TOther& other, std::enable_if_t().cx)> && std::is_integral_v().cy)>, int> /*sentinel*/ = 0) : + size(static_cast(other.cx), static_cast(other.cy)) + { + + } + + constexpr bool operator==(const size& other) const noexcept + { + return _width == other._width && + _height == other._height; + } + + constexpr bool operator!=(const size& other) const noexcept + { + return !(*this == other); + } + + size operator+(const size& other) const + { + ptrdiff_t width; + THROW_HR_IF(E_ABORT, !base::CheckAdd(_width, other._width).AssignIfValid(&width)); + + ptrdiff_t height; + THROW_HR_IF(E_ABORT, !base::CheckAdd(_height, other._height).AssignIfValid(&height)); + + return size{ width, height }; + } + + size operator-(const size& other) const + { + ptrdiff_t width; + THROW_HR_IF(E_ABORT, !base::CheckSub(_width, other._width).AssignIfValid(&width)); + + ptrdiff_t height; + THROW_HR_IF(E_ABORT, !base::CheckSub(_height, other._height).AssignIfValid(&height)); + + return size{ width, height }; + } + + size operator*(const size& other) const + { + ptrdiff_t width; + THROW_HR_IF(E_ABORT, !base::CheckMul(_width, other._width).AssignIfValid(&width)); + + ptrdiff_t height; + THROW_HR_IF(E_ABORT, !base::CheckMul(_height, other._height).AssignIfValid(&height)); + + return size{ width, height }; + } + + size operator/(const size& other) const + { + ptrdiff_t width; + THROW_HR_IF(E_ABORT, !base::CheckDiv(_width, other._width).AssignIfValid(&width)); + + ptrdiff_t height; + THROW_HR_IF(E_ABORT, !base::CheckDiv(_height, other._height).AssignIfValid(&height)); + + return size{ width, height}; + } + + size divide_ceil(const size& other) const + { + // Divide normally to get the floor. + const size floor = *this / other; + + ptrdiff_t adjWidth = 0; + ptrdiff_t adjHeight = 0; + + // Check for width remainder, anything not 0. + if (_width % other._width) + { + // If there was any remainder, + // Grow the magnitude by 1 in the + // direction of the sign. + if (floor.width() >= 0) + { + ++adjWidth; + } + else + { + --adjWidth; + } + } + + // Check for height remainder, anything not 0. + if (_height % other._height) + { + // If there was any remainder, + // Grow the magnitude by 1 in the + // direction of the sign. + if (_height >= 0) + { + ++adjHeight; + } + else + { + --adjHeight; + } + } + + return floor + size{ adjWidth, adjHeight }; + } + + constexpr ptrdiff_t width() const noexcept + { + return _width; + } + + template + T width() const + { + T ret; + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(width()).AssignIfValid(&ret)); + return ret; + } + + constexpr ptrdiff_t height() const noexcept + { + return _height; + } + + template + T height() const + { + T ret; + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(height()).AssignIfValid(&ret)); + return ret; + } + + ptrdiff_t area() const + { + ptrdiff_t result; + THROW_HR_IF(E_ABORT, !base::CheckMul(_width, _height).AssignIfValid(&result)); + return result; + } + +#ifdef _WINCONTYPES_ + operator COORD() const + { + COORD ret; + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(_width).AssignIfValid(&ret.X)); + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(_height).AssignIfValid(&ret.Y)); + return ret; + } +#endif + +#ifdef _WINDEF_ + operator SIZE() const + { + SIZE ret; + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(_width).AssignIfValid(&ret.cx)); + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(_height).AssignIfValid(&ret.cy)); + return ret; + } +#endif + +#ifdef DCOMMON_H_INCLUDED + constexpr operator D2D1_SIZE_F() const noexcept + { + return D2D1_SIZE_F{ gsl::narrow_cast(_width), gsl::narrow_cast(_height) }; + } +#endif + + protected: + ptrdiff_t _width; + ptrdiff_t _height; + +#ifdef UNIT_TESTING + friend class ::SizeTests; +#endif + }; +}; + +#ifdef __WEX_COMMON_H__ +namespace WEX::TestExecution +{ + template<> + class VerifyOutputTraits<::til::size> + { + public: + static WEX::Common::NoThrowString ToString(const ::til::size& size) + { + return WEX::Common::NoThrowString().Format(L"[W:%td, H:%td]", size.width(), size.height()); + } + }; + + template<> + class VerifyCompareTraits<::til::size, ::til::size> + { + public: + static bool AreEqual(const ::til::size& expected, const ::til::size& actual) + { + return expected == actual; + } + + static bool AreSame(const ::til::size& expected, const ::til::size& actual) + { + return &expected == &actual; + } + + static bool IsLessThan(const ::til::size& expectedLess, const ::til::size& expectedGreater) = delete; + + static bool IsGreaterThan(const ::til::size& expectedGreater, const ::til::size& expectedLess) = delete; + + static bool IsNull(const ::til::size& object) + { + return object == til::size{}; + } + }; +}; +#endif diff --git a/src/til/precomp.h b/src/til/precomp.h index 6e68de7f620..2375e15ec09 100644 --- a/src/til/precomp.h +++ b/src/til/precomp.h @@ -25,7 +25,17 @@ Module Name: // Windows Header Files: #include +#define BLOCK_TIL // This includes support libraries from the CRT, STL, WIL, and GSL #include "LibraryIncludes.h" +#include "WexTestClass.h" + +// Include DirectX common structures so we can test them. +// (before TIL so its support lights up) +#include + +// Include TIL after Wex to get test comparators. +#include "til.h" + // clang-format on diff --git a/src/til/ut_til/SizeTests.cpp b/src/til/ut_til/SizeTests.cpp new file mode 100644 index 00000000000..bc43bde3e61 --- /dev/null +++ b/src/til/ut_til/SizeTests.cpp @@ -0,0 +1,486 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "precomp.h" + +#include "til/size.h" + +using namespace WEX::Common; +using namespace WEX::Logging; +using namespace WEX::TestExecution; + +class SizeTests +{ + TEST_CLASS(SizeTests); + + TEST_METHOD(DefaultConstruct) + { + const til::size sz; + VERIFY_ARE_EQUAL(0, sz._width); + VERIFY_ARE_EQUAL(0, sz._height); + } + + TEST_METHOD(RawConstruct) + { + const til::size sz{ 5, 10 }; + VERIFY_ARE_EQUAL(5, sz._width); + VERIFY_ARE_EQUAL(10, sz._height); + } + + TEST_METHOD(UnsignedConstruct) + { + Log::Comment(L"0.) Normal unsigned construct."); + { + const size_t width = 5; + const size_t height = 10; + + const til::size sz{ width, height }; + VERIFY_ARE_EQUAL(5, sz._width); + VERIFY_ARE_EQUAL(10, sz._height); + } + + Log::Comment(L"1.) Unsigned construct overflow on width."); + { + constexpr size_t width = std::numeric_limits().max(); + const size_t height = 10; + + auto fn = [&]() + { + til::size sz{ width, height }; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Unsigned construct overflow on height."); + { + constexpr size_t height = std::numeric_limits().max(); + const size_t width = 10; + + auto fn = [&]() + { + til::size sz{ width, height }; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(SignedConstruct) + { + const ptrdiff_t width = -5; + const ptrdiff_t height = -10; + + const til::size sz{ width, height }; + VERIFY_ARE_EQUAL(width, sz._width); + VERIFY_ARE_EQUAL(height, sz._height); + } + + TEST_METHOD(CoordConstruct) + { + COORD coord{ -5, 10 }; + + const til::size sz{ coord }; + VERIFY_ARE_EQUAL(coord.X, sz._width); + VERIFY_ARE_EQUAL(coord.Y, sz._height); + } + + TEST_METHOD(SizeConstruct) + { + SIZE size{ 5, -10 }; + + const til::size sz{ size }; + VERIFY_ARE_EQUAL(size.cx, sz._width); + VERIFY_ARE_EQUAL(size.cy, sz._height); + } + + TEST_METHOD(Equality) + { + Log::Comment(L"0.) Equal."); + { + const til::size s1{ 5, 10 }; + const til::size s2{ 5, 10 }; + VERIFY_IS_TRUE(s1 == s2); + } + + Log::Comment(L"1.) Left Width changed."); + { + const til::size s1{ 4, 10 }; + const til::size s2{ 5, 10 }; + VERIFY_IS_FALSE(s1 == s2); + } + + Log::Comment(L"2.) Right Width changed."); + { + const til::size s1{ 5, 10 }; + const til::size s2{ 6, 10 }; + VERIFY_IS_FALSE(s1 == s2); + } + + Log::Comment(L"3.) Left Height changed."); + { + const til::size s1{ 5, 9 }; + const til::size s2{ 5, 10 }; + VERIFY_IS_FALSE(s1 == s2); + } + + Log::Comment(L"4.) Right Height changed."); + { + const til::size s1{ 5, 10 }; + const til::size s2{ 5, 11 }; + VERIFY_IS_FALSE(s1 == s2); + } + } + + TEST_METHOD(Inequality) + { + Log::Comment(L"0.) Equal."); + { + const til::size s1{ 5, 10 }; + const til::size s2{ 5, 10 }; + VERIFY_IS_FALSE(s1 != s2); + } + + Log::Comment(L"1.) Left Width changed."); + { + const til::size s1{ 4, 10 }; + const til::size s2{ 5, 10 }; + VERIFY_IS_TRUE(s1 != s2); + } + + Log::Comment(L"2.) Right Width changed."); + { + const til::size s1{ 5, 10 }; + const til::size s2{ 6, 10 }; + VERIFY_IS_TRUE(s1 != s2); + } + + Log::Comment(L"3.) Left Height changed."); + { + const til::size s1{ 5, 9 }; + const til::size s2{ 5, 10 }; + VERIFY_IS_TRUE(s1 != s2); + } + + Log::Comment(L"4.) Right Height changed."); + { + const til::size s1{ 5, 10 }; + const til::size s2{ 5, 11 }; + VERIFY_IS_TRUE(s1 != s2); + } + } + + TEST_METHOD(Addition) + { + Log::Comment(L"0.) Addition of two things that should be in bounds."); + { + const til::size sz{ 5,10 }; + const til::size sz2{ 23, 47 }; + + const til::size expected{ sz.width() + sz2.width(), sz.height() + sz2.height() }; + + VERIFY_ARE_EQUAL(expected, sz + sz2); + } + + Log::Comment(L"1.) Addition results in value that is too large (width)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::size sz{ bigSize, static_cast(0) }; + const til::size sz2{ 1, 1 }; + + auto fn = [&]() + { + sz + sz2; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Addition results in value that is too large (height)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::size sz{ static_cast(0), bigSize }; + const til::size sz2{ 1, 1 }; + + auto fn = [&]() + { + sz + sz2; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(Subtraction) + { + Log::Comment(L"0.) Subtraction of two things that should be in bounds."); + { + const til::size sz{ 5,10 }; + const til::size sz2{ 23, 47 }; + + const til::size expected{ sz.width() - sz2.width(), sz.height() - sz2.height() }; + + VERIFY_ARE_EQUAL(expected, sz - sz2); + } + + Log::Comment(L"1.) Subtraction results in value that is too small (width)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::size sz{ bigSize, static_cast(0) }; + const til::size sz2{ -2, -2 }; + + auto fn = [&]() + { + sz2 - sz; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Subtraction results in value that is too small (height)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::size sz{ static_cast(0), bigSize }; + const til::size sz2{ -2, -2 }; + + auto fn = [&]() + { + sz2 - sz; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(Multiplication) + { + Log::Comment(L"0.) Multiplication of two things that should be in bounds."); + { + const til::size sz{ 5,10 }; + const til::size sz2{ 23, 47 }; + + const til::size expected{ sz.width() * sz2.width(), sz.height() * sz2.height() }; + + VERIFY_ARE_EQUAL(expected, sz * sz2); + } + + Log::Comment(L"1.) Multiplication results in value that is too large (width)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::size sz{ bigSize, static_cast(0) }; + const til::size sz2{ 10, 10 }; + + auto fn = [&]() + { + sz * sz2; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Multiplication results in value that is too large (height)."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::size sz{ static_cast(0), bigSize }; + const til::size sz2{ 10, 10 }; + + auto fn = [&]() + { + sz * sz2; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(Division) + { + Log::Comment(L"0.) Division of two things that should be in bounds."); + { + const til::size sz{ 555, 510 }; + const til::size sz2{ 23, 47 }; + + const til::size expected{ sz.width() / sz2.width(), sz.height() / sz2.height() }; + + VERIFY_ARE_EQUAL(expected, sz / sz2); + } + + Log::Comment(L"1.) Division by zero"); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::size sz{ bigSize, static_cast(0) }; + const til::size sz2{ 1, 1 }; + + auto fn = [&]() + { + sz2 / sz; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(DivisionRoundingUp) + { + Log::Comment(L"1.) Division rounding up with positive result."); + { + const til::size sz{ 10, 5 }; + const til::size divisor{ 3, 2 }; + + // 10 / 3 is 3.333, rounded up is 4. + // 5 / 2 is 2.5, rounded up is 3. + const til::size expected{ 4, 3 }; + + VERIFY_ARE_EQUAL(expected, sz.divide_ceil(divisor)); + } + + Log::Comment(L"2.) Division rounding larger(up) with negative result."); + { + const til::size sz{ -10, -5 }; + const til::size divisor{ 3, 2 }; + + // -10 / 3 is -3.333, rounded up is -4. + // -5 / 2 is -2.5, rounded up is -3. + const til::size expected{ -4, -3 }; + + VERIFY_ARE_EQUAL(expected, sz.divide_ceil(divisor)); + } + } + + TEST_METHOD(Width) + { + const til::size sz{ 5,10 }; + VERIFY_ARE_EQUAL(sz._width, sz.width()); + } + + TEST_METHOD(WidthCast) + { + const til::size sz{ 5,10 }; + VERIFY_ARE_EQUAL(static_cast(sz._width), sz.width()); + } + + TEST_METHOD(Height) + { + const til::size sz{ 5,10 }; + VERIFY_ARE_EQUAL(sz._height, sz.height()); + } + + TEST_METHOD(HeightCast) + { + const til::size sz{ 5,10 }; + VERIFY_ARE_EQUAL(static_cast(sz._height), sz.height()); + } + + TEST_METHOD(Area) + { + Log::Comment(L"0.) Area of two things that should be in bounds."); + { + const til::size sz{ 5,10 }; + VERIFY_ARE_EQUAL(sz._width * sz._height, sz.area()); + } + + Log::Comment(L"1.) Area is out of bounds on multiplication."); + { + constexpr ptrdiff_t bigSize = std::numeric_limits().max(); + const til::size sz{ bigSize, bigSize}; + + auto fn = [&]() + { + sz.area(); + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(CastToCoord) + { + Log::Comment(L"0.) Typical situation."); + { + const til::size sz{ 5, 10 }; + COORD val = sz; + VERIFY_ARE_EQUAL(5, val.X); + VERIFY_ARE_EQUAL(10, val.Y); + } + + Log::Comment(L"1.) Overflow on width."); + { + constexpr ptrdiff_t width = std::numeric_limits().max(); + const ptrdiff_t height = 10; + const til::size sz{ width, height }; + + auto fn = [&]() + { + COORD val = sz; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Overflow on height."); + { + constexpr ptrdiff_t height = std::numeric_limits().max(); + const ptrdiff_t width = 10; + const til::size sz{ width, height }; + + auto fn = [&]() + { + COORD val = sz; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(CastToSize) + { + Log::Comment(L"0.) Typical situation."); + { + const til::size sz{ 5, 10 }; + SIZE val = sz; + VERIFY_ARE_EQUAL(5, val.cx); + VERIFY_ARE_EQUAL(10, val.cy); + } + + Log::Comment(L"1.) Overflow on width."); + { + constexpr ptrdiff_t width = std::numeric_limits().max(); + const ptrdiff_t height = 10; + const til::size sz{ width, height }; + + auto fn = [&]() + { + SIZE val = sz; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"2.) Overflow on height."); + { + constexpr ptrdiff_t height = std::numeric_limits().max(); + const ptrdiff_t width = 10; + const til::size sz{ width, height }; + + auto fn = [&]() + { + SIZE val = sz; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(CastToD2D1SizeF) + { + Log::Comment(L"0.) Typical situation."); + { + const til::size sz{ 5, 10 }; + D2D1_SIZE_F val = sz; + VERIFY_ARE_EQUAL(5, val.width); + VERIFY_ARE_EQUAL(10, val.height); + } + + // All ptrdiff_ts fit into a float, so there's no exception tests. + } +}; diff --git a/src/til/ut_til/SomeTests.cpp b/src/til/ut_til/SomeTests.cpp index 981a726cdb6..e7a1c02587d 100644 --- a/src/til/ut_til/SomeTests.cpp +++ b/src/til/ut_til/SomeTests.cpp @@ -2,7 +2,6 @@ // Licensed under the MIT license. #include "precomp.h" -#include "WexTestClass.h" using namespace WEX::Common; using namespace WEX::Logging; diff --git a/src/til/ut_til/til.unit.tests.vcxproj b/src/til/ut_til/til.unit.tests.vcxproj index 3245f172e72..8ce1de1772d 100644 --- a/src/til/ut_til/til.unit.tests.vcxproj +++ b/src/til/ut_til/til.unit.tests.vcxproj @@ -10,6 +10,7 @@ + Create diff --git a/src/til/ut_til/til.unit.tests.vcxproj.filters b/src/til/ut_til/til.unit.tests.vcxproj.filters new file mode 100644 index 00000000000..c78750fac19 --- /dev/null +++ b/src/til/ut_til/til.unit.tests.vcxproj.filters @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + \ No newline at end of file From abd0056ea5745f3baaf94b7a5965c54141422b57 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 9 Mar 2020 12:23:33 -0700 Subject: [PATCH 2/9] code format run. --- src/inc/til/size.h | 15 +++------ src/til/ut_til/SizeTests.cpp | 64 ++++++++++++++---------------------- 2 files changed, 30 insertions(+), 49 deletions(-) diff --git a/src/inc/til/size.h b/src/inc/til/size.h index 3940184485b..b47f70103f0 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -15,14 +15,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr size() noexcept : size(0, 0) { - } #if defined(_M_AMD64) || defined(_M_ARM64) constexpr size(int width, int height) noexcept : size(static_cast(width), static_cast(height)) { - } #endif @@ -36,27 +34,24 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _width(width), _height(height) { - } template constexpr size(const TOther& other, std::enable_if_t().X)> && std::is_integral_v().Y)>, int> /*sentinel*/ = 0) : size(static_cast(other.X), static_cast(other.Y)) { - } template constexpr size(const TOther& other, std::enable_if_t().cx)> && std::is_integral_v().cy)>, int> /*sentinel*/ = 0) : size(static_cast(other.cx), static_cast(other.cy)) { - } constexpr bool operator==(const size& other) const noexcept { return _width == other._width && - _height == other._height; + _height == other._height; } constexpr bool operator!=(const size& other) const noexcept @@ -105,7 +100,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" ptrdiff_t height; THROW_HR_IF(E_ABORT, !base::CheckDiv(_height, other._height).AssignIfValid(&height)); - return size{ width, height}; + return size{ width, height }; } size divide_ceil(const size& other) const @@ -119,7 +114,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // Check for width remainder, anything not 0. if (_width % other._width) { - // If there was any remainder, + // If there was any remainder, // Grow the magnitude by 1 in the // direction of the sign. if (floor.width() >= 0) @@ -135,7 +130,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // Check for height remainder, anything not 0. if (_height % other._height) { - // If there was any remainder, + // If there was any remainder, // Grow the magnitude by 1 in the // direction of the sign. if (_height >= 0) @@ -177,7 +172,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return ret; } - ptrdiff_t area() const + ptrdiff_t area() const { ptrdiff_t result; THROW_HR_IF(E_ABORT, !base::CheckMul(_width, _height).AssignIfValid(&result)); diff --git a/src/til/ut_til/SizeTests.cpp b/src/til/ut_til/SizeTests.cpp index bc43bde3e61..1ad276a2933 100644 --- a/src/til/ut_til/SizeTests.cpp +++ b/src/til/ut_til/SizeTests.cpp @@ -44,8 +44,7 @@ class SizeTests constexpr size_t width = std::numeric_limits().max(); const size_t height = 10; - auto fn = [&]() - { + auto fn = [&]() { til::size sz{ width, height }; }; @@ -57,8 +56,7 @@ class SizeTests constexpr size_t height = std::numeric_limits().max(); const size_t width = 10; - auto fn = [&]() - { + auto fn = [&]() { til::size sz{ width, height }; }; @@ -174,7 +172,7 @@ class SizeTests { Log::Comment(L"0.) Addition of two things that should be in bounds."); { - const til::size sz{ 5,10 }; + const til::size sz{ 5, 10 }; const til::size sz2{ 23, 47 }; const til::size expected{ sz.width() + sz2.width(), sz.height() + sz2.height() }; @@ -188,8 +186,7 @@ class SizeTests const til::size sz{ bigSize, static_cast(0) }; const til::size sz2{ 1, 1 }; - auto fn = [&]() - { + auto fn = [&]() { sz + sz2; }; @@ -202,8 +199,7 @@ class SizeTests const til::size sz{ static_cast(0), bigSize }; const til::size sz2{ 1, 1 }; - auto fn = [&]() - { + auto fn = [&]() { sz + sz2; }; @@ -215,7 +211,7 @@ class SizeTests { Log::Comment(L"0.) Subtraction of two things that should be in bounds."); { - const til::size sz{ 5,10 }; + const til::size sz{ 5, 10 }; const til::size sz2{ 23, 47 }; const til::size expected{ sz.width() - sz2.width(), sz.height() - sz2.height() }; @@ -229,8 +225,7 @@ class SizeTests const til::size sz{ bigSize, static_cast(0) }; const til::size sz2{ -2, -2 }; - auto fn = [&]() - { + auto fn = [&]() { sz2 - sz; }; @@ -243,8 +238,7 @@ class SizeTests const til::size sz{ static_cast(0), bigSize }; const til::size sz2{ -2, -2 }; - auto fn = [&]() - { + auto fn = [&]() { sz2 - sz; }; @@ -256,7 +250,7 @@ class SizeTests { Log::Comment(L"0.) Multiplication of two things that should be in bounds."); { - const til::size sz{ 5,10 }; + const til::size sz{ 5, 10 }; const til::size sz2{ 23, 47 }; const til::size expected{ sz.width() * sz2.width(), sz.height() * sz2.height() }; @@ -270,9 +264,8 @@ class SizeTests const til::size sz{ bigSize, static_cast(0) }; const til::size sz2{ 10, 10 }; - auto fn = [&]() - { - sz * sz2; + auto fn = [&]() { + sz* sz2; }; VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); @@ -284,9 +277,8 @@ class SizeTests const til::size sz{ static_cast(0), bigSize }; const til::size sz2{ 10, 10 }; - auto fn = [&]() - { - sz * sz2; + auto fn = [&]() { + sz* sz2; }; VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); @@ -311,8 +303,7 @@ class SizeTests const til::size sz{ bigSize, static_cast(0) }; const til::size sz2{ 1, 1 }; - auto fn = [&]() - { + auto fn = [&]() { sz2 / sz; }; @@ -349,25 +340,25 @@ class SizeTests TEST_METHOD(Width) { - const til::size sz{ 5,10 }; + const til::size sz{ 5, 10 }; VERIFY_ARE_EQUAL(sz._width, sz.width()); } TEST_METHOD(WidthCast) { - const til::size sz{ 5,10 }; + const til::size sz{ 5, 10 }; VERIFY_ARE_EQUAL(static_cast(sz._width), sz.width()); } TEST_METHOD(Height) { - const til::size sz{ 5,10 }; + const til::size sz{ 5, 10 }; VERIFY_ARE_EQUAL(sz._height, sz.height()); } TEST_METHOD(HeightCast) { - const til::size sz{ 5,10 }; + const til::size sz{ 5, 10 }; VERIFY_ARE_EQUAL(static_cast(sz._height), sz.height()); } @@ -375,17 +366,16 @@ class SizeTests { Log::Comment(L"0.) Area of two things that should be in bounds."); { - const til::size sz{ 5,10 }; + const til::size sz{ 5, 10 }; VERIFY_ARE_EQUAL(sz._width * sz._height, sz.area()); } Log::Comment(L"1.) Area is out of bounds on multiplication."); { constexpr ptrdiff_t bigSize = std::numeric_limits().max(); - const til::size sz{ bigSize, bigSize}; + const til::size sz{ bigSize, bigSize }; - auto fn = [&]() - { + auto fn = [&]() { sz.area(); }; @@ -409,8 +399,7 @@ class SizeTests const ptrdiff_t height = 10; const til::size sz{ width, height }; - auto fn = [&]() - { + auto fn = [&]() { COORD val = sz; }; @@ -423,8 +412,7 @@ class SizeTests const ptrdiff_t width = 10; const til::size sz{ width, height }; - auto fn = [&]() - { + auto fn = [&]() { COORD val = sz; }; @@ -448,8 +436,7 @@ class SizeTests const ptrdiff_t height = 10; const til::size sz{ width, height }; - auto fn = [&]() - { + auto fn = [&]() { SIZE val = sz; }; @@ -462,8 +449,7 @@ class SizeTests const ptrdiff_t width = 10; const til::size sz{ width, height }; - auto fn = [&]() - { + auto fn = [&]() { SIZE val = sz; }; From 016be0acf882fbba7890580cf9b76949e67e41a2 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 9 Mar 2020 13:56:30 -0700 Subject: [PATCH 3/9] add comment for why int cast one is blocked out --- src/inc/til/size.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/inc/til/size.h b/src/inc/til/size.h index b47f70103f0..d2b30d57c75 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -17,6 +17,9 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { } + // On 64-bit processors, int and ptrdiff_t are different fundamental types. + // On 32-bit processors, they're the same which makes this a double-definition + // with the `ptrdiff_t` one below. #if defined(_M_AMD64) || defined(_M_ARM64) constexpr size(int width, int height) noexcept : size(static_cast(width), static_cast(height)) From a34a85552bcd1819ebd3f1fad03179a141d4ecd2 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 9 Mar 2020 14:12:54 -0700 Subject: [PATCH 4/9] Fix error in test logic for x86 environments. --- src/til/ut_til/SizeTests.cpp | 42 +++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/src/til/ut_til/SizeTests.cpp b/src/til/ut_til/SizeTests.cpp index 1ad276a2933..09b57369813 100644 --- a/src/til/ut_til/SizeTests.cpp +++ b/src/til/ut_til/SizeTests.cpp @@ -430,30 +430,52 @@ class SizeTests VERIFY_ARE_EQUAL(10, val.cy); } - Log::Comment(L"1.) Overflow on width."); + Log::Comment(L"1.) Fit max width into SIZE (may overflow)."); { constexpr ptrdiff_t width = std::numeric_limits().max(); const ptrdiff_t height = 10; const til::size sz{ width, height }; - auto fn = [&]() { - SIZE val = sz; - }; + // On some platforms, ptrdiff_t will fit inside cx/cy + const bool overflowExpected = width > std::numeric_limits().max(); - VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + if (overflowExpected) + { + auto fn = [&]() { + SIZE val = sz; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + else + { + SIZE val = sz; + VERIFY_ARE_EQUAL(width, val.cx); + } } - Log::Comment(L"2.) Overflow on height."); + Log::Comment(L"2.) Fit max height into SIZE (may overflow)."); { constexpr ptrdiff_t height = std::numeric_limits().max(); const ptrdiff_t width = 10; const til::size sz{ width, height }; - auto fn = [&]() { - SIZE val = sz; - }; + // On some platforms, ptrdiff_t will fit inside cx/cy + const bool overflowExpected = height > std::numeric_limits().max(); - VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + if (overflowExpected) + { + auto fn = [&]() { + SIZE val = sz; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + else + { + SIZE val = sz; + VERIFY_ARE_EQUAL(height, val.cy); + } } } From 952d92a8851d185a459df7c766c8a46a585a1f0c Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 9 Mar 2020 14:16:13 -0700 Subject: [PATCH 5/9] SA pass. --- OpenConsole.sln | 3 ++- src/inc/til/size.h | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/OpenConsole.sln b/OpenConsole.sln index fc3eee7aa36..978bc75d9d1 100644 --- a/OpenConsole.sln +++ b/OpenConsole.sln @@ -1388,7 +1388,8 @@ Global {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|Any CPU.ActiveCfg = AuditMode|Win32 {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|ARM64.ActiveCfg = AuditMode|ARM64 {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|ARM64.Build.0 = AuditMode|ARM64 - {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|x64.ActiveCfg = Release|x64 + {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|x64.ActiveCfg = AuditMode|x64 + {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|x64.Build.0 = AuditMode|x64 {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|x86.ActiveCfg = AuditMode|Win32 {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|x86.Build.0 = AuditMode|Win32 {767268EE-174A-46FE-96F0-EEE698A1BBC9}.Debug|Any CPU.ActiveCfg = Debug|Win32 diff --git a/src/inc/til/size.h b/src/inc/til/size.h index d2b30d57c75..e1c275093d4 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -236,12 +236,12 @@ namespace WEX::TestExecution class VerifyCompareTraits<::til::size, ::til::size> { public: - static bool AreEqual(const ::til::size& expected, const ::til::size& actual) + static bool AreEqual(const ::til::size& expected, const ::til::size& actual) noexcept { return expected == actual; } - static bool AreSame(const ::til::size& expected, const ::til::size& actual) + static bool AreSame(const ::til::size& expected, const ::til::size& actual) noexcept { return &expected == &actual; } @@ -250,7 +250,7 @@ namespace WEX::TestExecution static bool IsGreaterThan(const ::til::size& expectedGreater, const ::til::size& expectedLess) = delete; - static bool IsNull(const ::til::size& object) + static bool IsNull(const ::til::size& object) noexcept { return object == til::size{}; } From e05d830861aa6e5438f6ec05f4fd12b340247d55 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 9 Mar 2020 14:22:12 -0700 Subject: [PATCH 6/9] Use multiplication to see if there was a remainder instead of modulo since we already did division (for perf reasons, per PR.) --- src/inc/til/size.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/inc/til/size.h b/src/inc/til/size.h index e1c275093d4..68d28dd1be0 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -115,7 +115,9 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" ptrdiff_t adjHeight = 0; // Check for width remainder, anything not 0. - if (_width % other._width) + // If we multiply the floored number with the other, it will equal + // the old width if there was no remainder. + if (other._width * floor._width != _width) { // If there was any remainder, // Grow the magnitude by 1 in the @@ -131,7 +133,9 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } // Check for height remainder, anything not 0. - if (_height % other._height) + // If we multiply the floored number with the other, it will equal + // the old width if there was no remainder. + if (other._height * floor._height != _height) { // If there was any remainder, // Grow the magnitude by 1 in the From 9192d9c4b1481ea155f21dea877b872bda83d049 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 9 Mar 2020 14:53:50 -0700 Subject: [PATCH 7/9] Revert OpenConsole.sln --- OpenConsole.sln | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/OpenConsole.sln b/OpenConsole.sln index 978bc75d9d1..fc3eee7aa36 100644 --- a/OpenConsole.sln +++ b/OpenConsole.sln @@ -1388,8 +1388,7 @@ Global {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|Any CPU.ActiveCfg = AuditMode|Win32 {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|ARM64.ActiveCfg = AuditMode|ARM64 {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|ARM64.Build.0 = AuditMode|ARM64 - {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|x64.ActiveCfg = AuditMode|x64 - {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|x64.Build.0 = AuditMode|x64 + {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|x64.ActiveCfg = Release|x64 {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|x86.ActiveCfg = AuditMode|Win32 {767268EE-174A-46FE-96F0-EEE698A1BBC9}.AuditMode|x86.Build.0 = AuditMode|Win32 {767268EE-174A-46FE-96F0-EEE698A1BBC9}.Debug|Any CPU.ActiveCfg = Debug|Win32 From b9da020fe8f118cd8606335c32021bf9ab951c56 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Tue, 10 Mar 2020 09:43:36 -0700 Subject: [PATCH 8/9] Add a natvis for til::size --- tools/ConsoleTypes.natvis | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/ConsoleTypes.natvis b/tools/ConsoleTypes.natvis index 4d793255ba3..a52cd374d3b 100644 --- a/tools/ConsoleTypes.natvis +++ b/tools/ConsoleTypes.natvis @@ -80,4 +80,8 @@ {{↓ wch:{_charData} mod:{_activeModifierKeys} repeat:{_repeatCount} vk:{_virtualKeyCode} vsc:{_virtualScanCode}} {{↑ wch:{_charData} mod:{_activeModifierKeys} repeat:{_repeatCount} vk:{_virtualKeyCode} vsc:{_virtualScanCode}} + + + {{W: {_width,d} x H: {_height,d} -> A: {_width * _height, d}}} + From 30064c198077664127e59571fe518ab7448ce23a Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Tue, 10 Mar 2020 13:31:05 -0700 Subject: [PATCH 9/9] nit: comment. --- src/inc/til/size.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/inc/til/size.h b/src/inc/til/size.h index 68d28dd1be0..f996c93251d 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -39,12 +39,14 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { } + // This template will convert to size from anything that has an X and a Y field that appear convertable to an integer value template constexpr size(const TOther& other, std::enable_if_t().X)> && std::is_integral_v().Y)>, int> /*sentinel*/ = 0) : size(static_cast(other.X), static_cast(other.Y)) { } + // This template will convert to size from anything that has a cx and a cy field that appear convertable to an integer value template constexpr size(const TOther& other, std::enable_if_t().cx)> && std::is_integral_v().cy)>, int> /*sentinel*/ = 0) : size(static_cast(other.cx), static_cast(other.cy))