From f8480b85ccc0c510d929805696937dacfc1ac0a9 Mon Sep 17 00:00:00 2001 From: skyline75489 Date: Fri, 10 Jan 2020 09:30:18 +0800 Subject: [PATCH] Use til::some to replace the SomeViewports class --- src/inc/til.h | 14 +------------- src/inc/til/at.h | 21 +++++++++++++++++++++ src/inc/til/some.h | 4 ++-- src/types/inc/viewport.hpp | 28 +++++----------------------- src/types/viewport.cpp | 14 ++++++++------ 5 files changed, 37 insertions(+), 44 deletions(-) create mode 100644 src/inc/til/at.h diff --git a/src/inc/til.h b/src/inc/til.h index ce30663be10..96746a16679 100644 --- a/src/inc/til.h +++ b/src/inc/til.h @@ -3,23 +3,11 @@ #pragma once +#include "til/at.h" #include "til/some.h" namespace til // Terminal Implementation Library. Also: "Today I Learned" { - // The at function declares that you've already sufficiently checked that your array access - // is in range before retrieving an item inside it at an offset. - // This is to save double/triple/quadruple testing in circumstances where you are already - // pivoting on the length of a set and now want to pull elements out of it by offset - // without checking again. - // gsl::at will do the check again. As will .at(). And using [] will have a warning in audit. - template - constexpr auto at(T& cont, const size_t i) -> decltype(cont[cont.size()]) - { -#pragma warning(suppress : 26482) // Suppress bounds.2 check for indexing with constant expressions -#pragma warning(suppress : 26446) // Suppress bounds.4 check for subscript operator. - return cont[i]; - } } // These sit outside the namespace because they sit outside for WIL too. diff --git a/src/inc/til/at.h b/src/inc/til/at.h new file mode 100644 index 00000000000..56001dfe879 --- /dev/null +++ b/src/inc/til/at.h @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +namespace til +{ + // The at function declares that you've already sufficiently checked that your array access + // is in range before retrieving an item inside it at an offset. + // This is to save double/triple/quadruple testing in circumstances where you are already + // pivoting on the length of a set and now want to pull elements out of it by offset + // without checking again. + // gsl::at will do the check again. As will .at(). And using [] will have a warning in audit. + template + constexpr auto at(T& cont, const size_t i) -> decltype(cont[cont.size()]) + { +#pragma warning(suppress : 26482) // Suppress bounds.2 check for indexing with constant expressions +#pragma warning(suppress : 26446) // Suppress bounds.4 check for subscript operator. + return cont[i]; + } +} diff --git a/src/inc/til/some.h b/src/inc/til/some.h index 9f1170968be..2f40da61c1b 100644 --- a/src/inc/til/some.h +++ b/src/inc/til/some.h @@ -154,7 +154,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _outOfRange(); } - _array[_used] = val; + til::at(_array, _used) = val; ++_used; } @@ -168,7 +168,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" --_used; - _array[_used] = 0; + til::at(_array, _used) = 0; } [[noreturn]] void _invalidArg() const diff --git a/src/types/inc/viewport.hpp b/src/types/inc/viewport.hpp index 25f5b9d7f66..711c5a02764 100644 --- a/src/types/inc/viewport.hpp +++ b/src/types/inc/viewport.hpp @@ -16,12 +16,16 @@ Author(s): namespace Microsoft::Console::Types { - struct SomeViewports; + class Viewport; + + using SomeViewports = til::some; class Viewport final { public: ~Viewport() {} + constexpr Viewport() noexcept : + _sr({ 0, 0, -1, -1 }){}; Viewport(const Viewport& other) noexcept; Viewport(Viewport&&) = default; Viewport& operator=(const Viewport&) & = default; @@ -125,28 +129,6 @@ namespace Microsoft::Console::Types friend class ViewportTests; #endif }; - - struct SomeViewports final - { - unsigned char used{ 0 }; - std::array viewports{ Viewport::Empty(), Viewport::Empty(), Viewport::Empty(), Viewport::Empty() }; - - // These two methods are to make this vaguely look like a std::vector. - - // Size is the number of viewports that are valid inside this structure - size_t size() const noexcept { return used; } - - // At retrieves a viewport at a particular index. If you retrieve beyond the valid size(), - // it will throw std::out_of_range - const Viewport& at(size_t index) const - { - if (index >= used) - { - throw std::out_of_range("Access attempted beyond valid size."); - } - return viewports.at(index); - } - }; } inline COORD operator-(const COORD& a, const COORD& b) noexcept diff --git a/src/types/viewport.cpp b/src/types/viewport.cpp index c6772bfdcc6..ae07de924c3 100644 --- a/src/types/viewport.cpp +++ b/src/types/viewport.cpp @@ -18,7 +18,7 @@ Viewport::Viewport(const Viewport& other) noexcept : Viewport Viewport::Empty() noexcept { - return Viewport({ 0, 0, -1, -1 }); + return Viewport(); } Viewport Viewport::FromInclusive(const SMALL_RECT sr) noexcept @@ -895,6 +895,7 @@ Viewport Viewport::ToOrigin() const noexcept // that was covered by `main` before the regional area of `removeMe` was taken out. // - You must check that each viewport .IsValid() before using it. [[nodiscard]] SomeViewports Viewport::Subtract(const Viewport& original, const Viewport& removeMe) noexcept +try { SomeViewports result; @@ -907,7 +908,7 @@ Viewport Viewport::ToOrigin() const noexcept if (!intersection.IsValid()) { // Just put the original rectangle into the results and return early. - result.viewports.at(result.used++) = original; + result.push_back(original); } // If the original rectangle matches the intersection, there is nothing to return. else if (original != intersection) @@ -1003,27 +1004,28 @@ Viewport Viewport::ToOrigin() const noexcept if (top.IsValid()) { - result.viewports.at(result.used++) = top; + result.push_back(top); } if (bottom.IsValid()) { - result.viewports.at(result.used++) = bottom; + result.push_back(bottom); } if (left.IsValid()) { - result.viewports.at(result.used++) = left; + result.push_back(left); } if (right.IsValid()) { - result.viewports.at(result.used++) = right; + result.push_back(right); } } return result; } +CATCH_FAIL_FAST() // Method Description: // - Returns true if the rectangle described by this Viewport has internal space