-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move to GSL 3.1.0 #6908
Merged
Merged
Move to GSL 3.1.0 #6908
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
Area-CodeHealth
Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc.
Issue-Task
It's a feature request, but it doesn't really need a major design.
Priority-2
A description (P2)
Product-Conhost
For issues in the Console codebase
Product-Terminal
The new Windows Terminal.
labels
Jul 14, 2020
DHowett
commented
Jul 14, 2020
table[13] = RGB(180, 0, 158); | ||
table[14] = RGB(97, 214, 214); | ||
table[15] = RGB(242, 242, 242); | ||
gsl::at(table, 0) = RGB(12, 12, 12); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
honestly, this should be an std::copy from a hardcoded table, lol
zadjii-msft
approved these changes
Jul 14, 2020
miniksa
reviewed
Jul 14, 2020
miniksa
approved these changes
Jul 14, 2020
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) * 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
DHowett
force-pushed
the
dev/duhowett/gsl3
branch
from
July 14, 2020 18:00
60d30a3
to
7951030
Compare
DHowett
added
the
AutoMerge
Marked for automatic merge by the bot when requirements are met
label
Jul 14, 2020
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
deleted the
dev/duhowett/gsl3
branch
July 14, 2020 18:31
DHowett
added a commit
that referenced
this pull request
Aug 5, 2020
- Move to GSL 3.1.0 (GH-6908) - Replace the color table init code with two const arrays (GH-6913) - Replace basic_string_view<T> with span<const T> (GH-6921) - Replace gsl::at with a new til::at(span) for pre-checked bounds (GH-6925) Related work items: MSFT:27866336
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Area-CodeHealth
Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc.
AutoMerge
Marked for automatic merge by the bot when requirements are met
Issue-Task
It's a feature request, but it doesn't really need a major design.
Priority-2
A description (P2)
Product-Conhost
For issues in the Console codebase
Product-Terminal
The new Windows Terminal.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 mimicsC++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 ofspan::at
with
gsl::at(x)
span::size_type
has finally given up onptrdiff_t
and becomesize_t
like all other containersWhile I was here, I also made the following mechanical replacements:
has_value
andvalue
back-to-back. Eachvalue
incurs anadditional presence test.
x.value().member
->x->member
(optional::operator->
skips the presence test)
x.value()
->*x
(as above)size_t
forsize_type
.gsl::narrow<size_t>(x.size())
->x.size()
gsl::narrow<ptrdiff_t>(nonSpan.size())
->nonSpan.size()
during span construction
I also replaced two instances of
x[x.size() - 1]
withx.back()
andone 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 spanlibrary.
Fixes #6251