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 BYTE_TYPE with a solution based on namespaces #1201

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

beinhaerter
Copy link
Contributor

  • A macro with the very generic name BYTE_TYPE is likely to collide with existing code, so get rid of the macro.
  • The new solution is to provide a non-deprecated byte in the namespace gsl::impl.
    • Users of GSL should use gsl::byte, which is still deprecated when mapped to a std::std::byte.
    • GSL types and functions need to use gsl::impl::byte so they do not trigger the deprecation warning.
  • The gsl::impl::byte return type in an exported function is not nice, it might mislead users to use that type in their own declarations. But the BYTE_TYPE solution is not better in this respect.

- A macro with the very generic name `BYTE_TYPE` is likely to collide with existing code, so get rid of the macro.
- The new solution is to provide a non-deprecated `byte` in the namespace `gsl::impl`.
  - Users of GSL should use `gsl::byte`, which is still deprecated when mapped to a `std::std::byte`.
  - GSL types and functions need to use `gsl::impl::byte` so they do not trigger the deprecation warning.
- The `gsl::impl::byte` return type in an exported function is not nice, it might mislead users to use that type in their own declarations. But the `BYTE_TYPE` solution is not better in this respect.
@carsonRadtke
Copy link
Collaborator

Removing the macro and keeping everything in the gsl namespace looks much better. Thanks!

@carsonRadtke carsonRadtke merged commit 2828399 into microsoft:main Feb 28, 2025
83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants