Repository navigation
Replace basic_string_view<T> with span<const T> - #6921
Merged
1 commit merged intoJul 15, 2020
Merged
1 commit merged into
1 commit merged into
Conversation
We were using std::basic_string_view as a stand-in for std::span so that we could change over all at once when C++20 dropped with full span support. That day's not here yet, but as of 54a7fce we're using GSL 3, whose span is C++20-compliant. This commit replaces every instance of basic_string_view that was not referring to an actual string with a span of the appropriate type. I moved the `const` qualifier into span's `T` because while `basic_string_view.at()` returns `const T&`, `span.at()` returns `T&` (without the const). I wanted to maintain the invariant that members of the span were immutable. * Mechanical Changes * `sv.at(x)` -> `gsl::at(sp, x)` I had to replace a `std::basic_string<>` with a `std::vector>` in ConImeInfo, and I chose to replace a manual array walk in ScreenInfoUiaProviderBase with a ranged-for. Please review those specifically. This will almost certainly cause a code size regression in Windows because I'm blowing out all the PGO counts. Whoops. Related: #3956, #975.
6 tasks done
Mike Griese (zadjii-msft)
approved these changes
Jul 15, 2020
Mike Griese (zadjii-msft)
added
the
Area-CodeHealth
| SAFEARRAY* psa = SafeArrayCreateVector(VT_I4, 0, gsl::narrow<ULONG>(data.size())); | ||
| if (psa != nullptr) | ||
| { | ||
| for (size_t i = 0; i < data.size(); i++) |
Contributor
There was a problem hiding this comment.
Well that was obviously a past brain fart over using the foreach loop with incrementing lIndex. Sheesh.
Member
Author
There was a problem hiding this comment.
yup! and we only caught it because i had to make an at replacement lol
Michael Niksa (miniksa)
approved these changes
Jul 15, 2020
Michael Niksa (miniksa)
left a comment
Michael Niksa (miniksa)
left a comment
Contributor
There was a problem hiding this comment.
Thank you. Using actual spans is way better.
Dustin L. Howett (DHowett)
added
the
AutoMerge
|
Hello Dustin L. Howett (@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/spandex
branch
July 15, 2020 17:00
Dustin L. Howett (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.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We were using std::basic_string_view as a stand-in for std::span so that
we could change over all at once when C++20 dropped with full span
support. That day's not here yet, but as of 54a7fce we're using GSL 3,
whose span is C++20-compliant.
This commit replaces every instance of basic_string_view that was not
referring to an actual string with a span of the appropriate type.
I moved the
constqualifier into span'sTbecause whilebasic_string_view.at()returnsconst T&,span.at()returnsT&(without the const). I wanted to maintain the invariant that members of
the span were immutable.
sv.at(x)->gsl::at(sp, x)sv.c{begin,end}->sp.{begin,end}(span's iterators are const)I had to replace a
std::basic_string<>with astd::vector<>inConImeInfo, and I chose to replace a manual array walk in
ScreenInfoUiaProviderBase with a ranged-for. Please review those
specifically.
This will almost certainly cause a code size regression in Windows
because I'm blowing out all the PGO counts. Whoops.
Related: #3956, #975.