Turn all these old-school pointer/size things into std::wstring_view and std::string_view throughout the parser/adapter
#975
Labels
Area-CodeHealth
Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc.
Area-VT
Virtual Terminal sequence support
Issue-Task
It's a feature request, but it doesn't really need a major design.
Product-Conhost
For issues in the Console codebase
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Comments
ghost
added
Needs-Triage
5 tasks
zadjii-msft
added
Area-CodeHealth
|
The link is 404. |
|
Uhg, thanks for catching that. Dunno why github horked it there. The original issue with this discussion was #891 |
DHowett-MSFT
removed
the
Needs-Triage
|
Hey look, this was actually fixed by #3956 |
zadjii-msft
added
Resolution-Fix-Committed
Nice catch. |
DHowett
added a commit
that referenced
this issue
Jul 14, 2020
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.
DHowett
added a commit
that referenced
this issue
Jul 15, 2020
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)` * `sv.c{begin,end}` -> `sp.{begin,end}` (span's iterators are const) 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Yuck. We should file a follow on task to turn all these old-school pointer/size things into
std::wstring_viewandstd::string_viewthroughout the parser/adapter...Originally posted by @miniksa in https://github.com/microsoft/terminal/diffs
The text was updated successfully, but these errors were encountered: