Skip to content

Commit

Permalink
Use 32-bit coordinates throughout the project (#13025)
Browse files Browse the repository at this point in the history
Previously this project used a great variety of types to present text buffer
coordinates: `short`, `unsigned short`, `int`, `unsigned int`, `size_t`,
`ptrdiff_t`, `COORD`/`SMALL_RECT` (aka `short`), and more.
This massive commit migrates almost all use of those types over to the
centralized types `til::point`/`size`/`rect`/`inclusive_rect` and their
underlying type `til::CoordType` (aka `int32_t`).

Due to the size of the changeset and statistics I expect it to contain bugs.
The biggest risk I see is that some code potentially, maybe implicitly, expected
arithmetic to be mod 2^16 and that this code now allows it to be mod 2^32.
Any narrowing into `short` later on would then throw exceptions.

## PR Checklist
* [x] Closes #4015
* [x] I work here
* [x] Tests added/passed

## Validation Steps Performed
Casual usage of OpenConsole and Windows Terminal. ✅
  • Loading branch information
lhecker committed Jun 3, 2022
1 parent c157f63 commit ed27737
Show file tree
Hide file tree
Showing 237 changed files with 3,610 additions and 3,906 deletions.
18 changes: 9 additions & 9 deletions src/buffer/out/AttrRow.cpp
Expand Up @@ -11,8 +11,8 @@
// - attr - the default text attribute
// Return Value:
// - constructed object
ATTR_ROW::ATTR_ROW(const uint16_t width, const TextAttribute attr) :
_data(width, attr) {}
ATTR_ROW::ATTR_ROW(const til::CoordType width, const TextAttribute attr) :
_data(gsl::narrow_cast<uint16_t>(width), attr) {}

// Routine Description:
// - Sets all properties of the ATTR_ROW to default values
Expand All @@ -32,9 +32,9 @@ void ATTR_ROW::Reset(const TextAttribute attr)
// - newWidth - The new width of the row.
// Return Value:
// - <none>, throws exceptions on failures.
void ATTR_ROW::Resize(const uint16_t newWidth)
void ATTR_ROW::Resize(const til::CoordType newWidth)
{
_data.resize_trailing_extent(newWidth);
_data.resize_trailing_extent(gsl::narrow<uint16_t>(newWidth));
}

// Routine Description:
Expand All @@ -45,9 +45,9 @@ void ATTR_ROW::Resize(const uint16_t newWidth)
// - the text attribute at column
// Note:
// - will throw on error
TextAttribute ATTR_ROW::GetAttrByColumn(const uint16_t column) const
TextAttribute ATTR_ROW::GetAttrByColumn(const til::CoordType column) const
{
return _data.at(column);
return _data.at(gsl::narrow<uint16_t>(column));
}

// Routine Description:
Expand All @@ -74,7 +74,7 @@ std::vector<uint16_t> ATTR_ROW::GetHyperlinks() const
// - attr - Attribute (color) to fill remaining characters with
// Return Value:
// - <none>
bool ATTR_ROW::SetAttrToEnd(const uint16_t beginIndex, const TextAttribute attr)
bool ATTR_ROW::SetAttrToEnd(const til::CoordType beginIndex, const TextAttribute attr)
{
_data.replace(gsl::narrow<uint16_t>(beginIndex), _data.size(), attr);
return true;
Expand Down Expand Up @@ -103,9 +103,9 @@ void ATTR_ROW::ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAtt
// - newAttr: The attribute to merge into this row.
// Return Value:
// - <none>
void ATTR_ROW::Replace(const uint16_t beginIndex, const uint16_t endIndex, const TextAttribute& newAttr)
void ATTR_ROW::Replace(const til::CoordType beginIndex, const til::CoordType endIndex, const TextAttribute& newAttr)
{
_data.replace(beginIndex, endIndex, newAttr);
_data.replace(gsl::narrow<uint16_t>(beginIndex), gsl::narrow<uint16_t>(endIndex), newAttr);
}

ATTR_ROW::const_iterator ATTR_ROW::begin() const noexcept
Expand Down
10 changes: 5 additions & 5 deletions src/buffer/out/AttrRow.hpp
Expand Up @@ -30,7 +30,7 @@ class ATTR_ROW final
public:
using const_iterator = rle_vector::const_iterator;

ATTR_ROW(uint16_t width, TextAttribute attr);
ATTR_ROW(til::CoordType width, TextAttribute attr);

~ATTR_ROW() = default;

Expand All @@ -40,13 +40,13 @@ class ATTR_ROW final
noexcept = default;
ATTR_ROW& operator=(ATTR_ROW&&) noexcept = default;

TextAttribute GetAttrByColumn(uint16_t column) const;
TextAttribute GetAttrByColumn(til::CoordType column) const;
std::vector<uint16_t> GetHyperlinks() const;

bool SetAttrToEnd(uint16_t beginIndex, TextAttribute attr);
bool SetAttrToEnd(til::CoordType beginIndex, TextAttribute attr);
void ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAttribute& replaceWith);
void Resize(uint16_t newWidth);
void Replace(uint16_t beginIndex, uint16_t endIndex, const TextAttribute& newAttr);
void Resize(til::CoordType newWidth);
void Replace(til::CoordType beginIndex, til::CoordType endIndex, const TextAttribute& newAttr);

const_iterator begin() const noexcept;
const_iterator end() const noexcept;
Expand Down
44 changes: 22 additions & 22 deletions src/buffer/out/CharRow.cpp
Expand Up @@ -17,7 +17,7 @@
// Note: will through if unable to allocate char/attribute buffers
#pragma warning(push)
#pragma warning(disable : 26447) // small_vector's constructor says it can throw but it should not given how we use it. This suppresses this error for the AuditMode build.
CharRow::CharRow(size_t rowWidth, ROW* const pParent) noexcept :
CharRow::CharRow(til::CoordType rowWidth, ROW* const pParent) noexcept :
_data(rowWidth, value_type()),
_pParent{ FAIL_FAST_IF_NULL(pParent) }
{
Expand All @@ -30,9 +30,9 @@ CharRow::CharRow(size_t rowWidth, ROW* const pParent) noexcept :
// - <none>
// Return Value:
// - the size of the row
size_t CharRow::size() const noexcept
til::CoordType CharRow::size() const noexcept
{
return _data.size();
return gsl::narrow_cast<til::CoordType>(_data.size());
}

// Routine Description:
Expand All @@ -55,7 +55,7 @@ void CharRow::Reset() noexcept
// - newSize - the new width of the character and attributes rows
// Return Value:
// - S_OK on success, otherwise relevant error code
[[nodiscard]] HRESULT CharRow::Resize(const size_t newSize) noexcept
[[nodiscard]] HRESULT CharRow::Resize(const til::CoordType newSize) noexcept
{
try
{
Expand Down Expand Up @@ -93,14 +93,14 @@ typename CharRow::const_iterator CharRow::cend() const noexcept
// - <none>
// Return Value:
// - The calculated left boundary of the internal string.
size_t CharRow::MeasureLeft() const noexcept
til::CoordType CharRow::MeasureLeft() const noexcept
{
auto it = _data.cbegin();
while (it != _data.cend() && it->IsSpace())
{
++it;
}
return it - _data.cbegin();
return gsl::narrow_cast<til::CoordType>(it - _data.cbegin());
}

// Routine Description:
Expand All @@ -109,17 +109,17 @@ size_t CharRow::MeasureLeft() const noexcept
// - <none>
// Return Value:
// - The calculated right boundary of the internal string.
size_t CharRow::MeasureRight() const
til::CoordType CharRow::MeasureRight() const
{
auto it = _data.crbegin();
while (it != _data.crend() && it->IsSpace())
{
++it;
}
return _data.crend() - it;
return gsl::narrow_cast<til::CoordType>(_data.crend() - it);
}

void CharRow::ClearCell(const size_t column)
void CharRow::ClearCell(const til::CoordType column)
{
_data.at(column).Reset();
}
Expand Down Expand Up @@ -149,7 +149,7 @@ bool CharRow::ContainsText() const noexcept
// Return Value:
// - the attribute
// Note: will throw exception if column is out of bounds
const DbcsAttribute& CharRow::DbcsAttrAt(const size_t column) const
const DbcsAttribute& CharRow::DbcsAttrAt(const til::CoordType column) const
{
return _data.at(column).DbcsAttr();
}
Expand All @@ -161,7 +161,7 @@ const DbcsAttribute& CharRow::DbcsAttrAt(const size_t column) const
// Return Value:
// - the attribute
// Note: will throw exception if column is out of bounds
DbcsAttribute& CharRow::DbcsAttrAt(const size_t column)
DbcsAttribute& CharRow::DbcsAttrAt(const til::CoordType column)
{
return _data.at(column).DbcsAttr();
}
Expand All @@ -173,7 +173,7 @@ DbcsAttribute& CharRow::DbcsAttrAt(const size_t column)
// Return Value:
// - <none>
// Note: will throw exception if column is out of bounds
void CharRow::ClearGlyph(const size_t column)
void CharRow::ClearGlyph(const til::CoordType column)
{
_data.at(column).EraseChars();
}
Expand All @@ -185,9 +185,9 @@ void CharRow::ClearGlyph(const size_t column)
// Return Value:
// - text data at column
// - Note: will throw exception if column is out of bounds
const CharRow::reference CharRow::GlyphAt(const size_t column) const
const CharRow::reference CharRow::GlyphAt(const til::CoordType column) const
{
THROW_HR_IF(E_INVALIDARG, column >= _data.size());
THROW_HR_IF(E_INVALIDARG, column < 0 || column >= gsl::narrow_cast<til::CoordType>(_data.size()));
return { const_cast<CharRow&>(*this), column };
}

Expand All @@ -198,9 +198,9 @@ const CharRow::reference CharRow::GlyphAt(const size_t column) const
// Return Value:
// - text data at column
// - Note: will throw exception if column is out of bounds
CharRow::reference CharRow::GlyphAt(const size_t column)
CharRow::reference CharRow::GlyphAt(const til::CoordType column)
{
THROW_HR_IF(E_INVALIDARG, column >= _data.size());
THROW_HR_IF(E_INVALIDARG, column < 0 || column >= gsl::narrow_cast<til::CoordType>(_data.size()));
return { *this, column };
}

Expand All @@ -209,7 +209,7 @@ std::wstring CharRow::GetText() const
std::wstring wstr;
wstr.reserve(_data.size());

for (size_t i = 0; i < _data.size(); ++i)
for (til::CoordType i = 0; i < gsl::narrow_cast<til::CoordType>(_data.size()); ++i)
{
const auto glyph = GlyphAt(i);
if (!DbcsAttrAt(i).IsTrailing())
Expand All @@ -231,9 +231,9 @@ std::wstring CharRow::GetText() const
// - wordDelimiters: the delimiters defined as a part of the DelimiterClass::DelimiterChar
// Return Value:
// - the delimiter class for the given char
const DelimiterClass CharRow::DelimiterClassAt(const size_t column, const std::wstring_view wordDelimiters) const
const DelimiterClass CharRow::DelimiterClassAt(const til::CoordType column, const std::wstring_view wordDelimiters) const
{
THROW_HR_IF(E_INVALIDARG, column >= _data.size());
THROW_HR_IF(E_INVALIDARG, column < 0 || column >= gsl::narrow_cast<til::CoordType>(_data.size()));

const auto glyph = *GlyphAt(column).begin();
if (glyph <= UNICODE_SPACE)
Expand Down Expand Up @@ -265,10 +265,10 @@ const UnicodeStorage& CharRow::GetUnicodeStorage() const noexcept
// Arguments:
// - column - the column to generate the key for
// Return Value:
// - the COORD key for data access from UnicodeStorage for the column
COORD CharRow::GetStorageKey(const size_t column) const noexcept
// - the til::point key for data access from UnicodeStorage for the column
til::point CharRow::GetStorageKey(const til::CoordType column) const noexcept
{
return { gsl::narrow<SHORT>(column), _pParent->GetId() };
return { column, _pParent->GetId() };
}

// Routine Description:
Expand Down
26 changes: 13 additions & 13 deletions src/buffer/out/CharRow.hpp
Expand Up @@ -54,22 +54,22 @@ class CharRow final
using const_reverse_iterator = typename boost::container::small_vector_base<value_type>::const_reverse_iterator;
using reference = typename CharRowCellReference;

CharRow(size_t rowWidth, ROW* const pParent) noexcept;
CharRow(til::CoordType rowWidth, ROW* const pParent) noexcept;

size_t size() const noexcept;
[[nodiscard]] HRESULT Resize(const size_t newSize) noexcept;
size_t MeasureLeft() const noexcept;
size_t MeasureRight() const;
til::CoordType size() const noexcept;
[[nodiscard]] HRESULT Resize(const til::CoordType newSize) noexcept;
til::CoordType MeasureLeft() const noexcept;
til::CoordType MeasureRight() const;
bool ContainsText() const noexcept;
const DbcsAttribute& DbcsAttrAt(const size_t column) const;
DbcsAttribute& DbcsAttrAt(const size_t column);
void ClearGlyph(const size_t column);
const DbcsAttribute& DbcsAttrAt(const til::CoordType column) const;
DbcsAttribute& DbcsAttrAt(const til::CoordType column);
void ClearGlyph(const til::CoordType column);

const DelimiterClass DelimiterClassAt(const size_t column, const std::wstring_view wordDelimiters) const;
const DelimiterClass DelimiterClassAt(const til::CoordType column, const std::wstring_view wordDelimiters) const;

// working with glyphs
const reference GlyphAt(const size_t column) const;
reference GlyphAt(const size_t column);
const reference GlyphAt(const til::CoordType column) const;
reference GlyphAt(const til::CoordType column);

// iterators
iterator begin() noexcept;
Expand All @@ -82,7 +82,7 @@ class CharRow final

UnicodeStorage& GetUnicodeStorage() noexcept;
const UnicodeStorage& GetUnicodeStorage() const noexcept;
COORD GetStorageKey(const size_t column) const noexcept;
til::point GetStorageKey(const til::CoordType column) const noexcept;

void UpdateParent(ROW* const pParent);

Expand All @@ -91,7 +91,7 @@ class CharRow final

private:
void Reset() noexcept;
void ClearCell(const size_t column);
void ClearCell(const til::CoordType column);
std::wstring GetText() const;

protected:
Expand Down
4 changes: 2 additions & 2 deletions src/buffer/out/CharRowCellReference.hpp
Expand Up @@ -25,7 +25,7 @@ class CharRowCellReference final
public:
using const_iterator = const wchar_t*;

CharRowCellReference(CharRow& parent, const size_t index) noexcept :
CharRowCellReference(CharRow& parent, const til::CoordType index) noexcept :
_parent{ parent },
_index{ index }
{
Expand All @@ -51,7 +51,7 @@ class CharRowCellReference final
// what char row the object belongs to
CharRow& _parent;
// the index of the cell in the parent char row
const size_t _index;
til::CoordType _index;

CharRowCell& _cellData();
const CharRowCell& _cellData() const;
Expand Down
8 changes: 4 additions & 4 deletions src/buffer/out/LineRendition.hpp
Expand Up @@ -21,16 +21,16 @@ enum class LineRendition
DoubleHeightBottom
};

constexpr SMALL_RECT ScreenToBufferLine(const SMALL_RECT& line, const LineRendition lineRendition)
constexpr til::inclusive_rect ScreenToBufferLine(const til::inclusive_rect& line, const LineRendition lineRendition)
{
// Use shift right to quickly divide the Left and Right by 2 for double width lines.
const SHORT scale = lineRendition == LineRendition::SingleWidth ? 0 : 1;
const auto scale = lineRendition == LineRendition::SingleWidth ? 0 : 1;
return { line.Left >> scale, line.Top, line.Right >> scale, line.Bottom };
}

constexpr SMALL_RECT BufferToScreenLine(const SMALL_RECT& line, const LineRendition lineRendition)
constexpr til::inclusive_rect BufferToScreenLine(const til::inclusive_rect& line, const LineRendition lineRendition)
{
// Use shift left to quickly multiply the Left and Right by 2 for double width lines.
const SHORT scale = lineRendition == LineRendition::SingleWidth ? 0 : 1;
const auto scale = lineRendition == LineRendition::SingleWidth ? 0 : 1;
return { line.Left << scale, line.Top, (line.Right << scale) + scale, line.Bottom };
}
8 changes: 4 additions & 4 deletions src/buffer/out/OutputCellIterator.cpp
Expand Up @@ -531,16 +531,16 @@ OutputCellView OutputCellIterator::s_GenerateView(const OutputCell& cell)
// - Gets the distance between two iterators relative to the input data given in.
// Return Value:
// - The number of items of the input run consumed between these two iterators.
ptrdiff_t OutputCellIterator::GetInputDistance(OutputCellIterator other) const noexcept
til::CoordType OutputCellIterator::GetInputDistance(OutputCellIterator other) const noexcept
{
return _pos - other._pos;
return gsl::narrow_cast<til::CoordType>(_pos - other._pos);
}

// Routine Description:
// - Gets the distance between two iterators relative to the number of cells inserted.
// Return Value:
// - The number of cells in the backing buffer filled between these two iterators.
ptrdiff_t OutputCellIterator::GetCellDistance(OutputCellIterator other) const noexcept
til::CoordType OutputCellIterator::GetCellDistance(OutputCellIterator other) const noexcept
{
return _distance - other._distance;
return gsl::narrow_cast<til::CoordType>(_distance - other._distance);
}
8 changes: 4 additions & 4 deletions src/buffer/out/OutputCellIterator.hpp
Expand Up @@ -29,7 +29,7 @@ class OutputCellIterator final
public:
using iterator_category = std::input_iterator_tag;
using value_type = OutputCellView;
using difference_type = ptrdiff_t;
using difference_type = til::CoordType;
using pointer = OutputCellView*;
using reference = OutputCellView&;

Expand All @@ -48,9 +48,9 @@ class OutputCellIterator final

operator bool() const noexcept;

ptrdiff_t GetCellDistance(OutputCellIterator other) const noexcept;
ptrdiff_t GetInputDistance(OutputCellIterator other) const noexcept;
friend ptrdiff_t operator-(OutputCellIterator one, OutputCellIterator two) = delete;
til::CoordType GetCellDistance(OutputCellIterator other) const noexcept;
til::CoordType GetInputDistance(OutputCellIterator other) const noexcept;
friend til::CoordType operator-(OutputCellIterator one, OutputCellIterator two) = delete;

OutputCellIterator& operator++();
OutputCellIterator operator++(int);
Expand Down

0 comments on commit ed27737

Please sign in to comment.