Skip to content
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

Replace gsl::byte/span with std #14763

Merged
merged 3 commits into from
Feb 2, 2023
Merged

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jan 30, 2023

This is a rather trivial changeset. Now that these two are present in the
std namespace there's no reason for us to continue using the gsl ones.
Additionally this ensures future compatibility with other 3rd party libraries.

@lhecker lhecker added the Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. label Jan 30, 2023
static constexpr VTParameter defaultParameter;
static constexpr std::span defaultParameters{ &defaultParameter, 1 };

std::span<const VTParameter> _values;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a more significant change, as the [] operator of std::span is not considered safe anymore (unlike gsl::span it doesn't fail-fast). When fixing it with til::at I noticed that for_each can be simplified with a for-range loop.

return cont[i];
}
#pragma warning(suppress : 26445) // Suppress lifetime check for a reference to std::span or std::string_view
return cont[i];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike gsl::span, std::span has an unchecked [] operator, so the extra if constexpr isn't necessary anymore.

BTW this made me realize that til::at might be a bit poorly named... I think we should rename it as til::at_unchecked or something, so that we can add a til::at_failfast and similar (and to make the intent clearer to future maintainers).

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed in an unusual way, but I got all 97.

src/terminal/adapter/DispatchTypes.hpp Show resolved Hide resolved
@DHowett
Copy link
Member

DHowett commented Jan 31, 2023

For future reviewers: Leonard is not lying when he says it's all mechanical. If you review only one file, make it DispatchTypes.cpp

@carlos-zamora carlos-zamora merged commit 42e8de3 into main Feb 2, 2023
@carlos-zamora carlos-zamora deleted the dev/lhecker/gsl-span-byte branch February 2, 2023 21:24
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants