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

Add __may_alias__ attribute to gsl::byte #668

Merged
merged 1 commit into from
May 1, 2018
Merged

Add __may_alias__ attribute to gsl::byte #668

merged 1 commit into from
May 1, 2018

Conversation

StephanDollberg
Copy link
Contributor

@StephanDollberg StephanDollberg commented Apr 21, 2018

C++20 defines std::byte* to have the same aliasing properties as
char*. Hence, we mark it with the __may_alias__ attribute under gcc
and clang.

Fixes #663

(Actually couldn't find whether this requirement is already in C++17 or not but gcc and clang already implement this in the compiler, trusting Niall on this part)

@StephanDollberg
Copy link
Contributor Author

Ugh, apparently there is a bug in gcc < 6 with attributes on enum classes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43407

@annagrin
Copy link

@StephanDollberg Are there any tests possible to show correct code gen?

@StephanDollberg
Copy link
Contributor Author

Had a quick look yesterday. There is already a test for aliasing but currently the tests are also being compiled with -fno-strict-aliasing. Though they don't fail if one removes that.

Will have another look today or tomorrow.

@martinmoene
Copy link
Contributor

@StephanDollberg

(Actually couldn't find whether this requirement is already in C++17 or not but gcc and clang already implement this in the compiler, trusting Niall on this part)

See this remark at issue #663

Related: PR #390

@StephanDollberg
Copy link
Contributor Author

StephanDollberg commented Apr 26, 2018 via email

C++17 defines `std::byte*` to have the same aliasing properties as
`char*`. Hence, we mark it with the `__may_alias__` attribute under gcc
and clang.

Based on the fix by Martin Moene in byte-lite.

Fixes #663
@StephanDollberg
Copy link
Contributor Author

@annagrin Checked with the tests. So the other day I was compiling with clang which apparently doesn't warn. GCC requires -fstrict-aliasing or -O2 (which turns strict aliasing on) to warn about the aliasing test.

Given that gcc 5 doesn't support the attribute I'd recommend against turning those flags on for now.

Rebased as well.

@annagrin
Copy link

annagrin commented May 1, 2018

Thanks @StephanDollberg!

@annagrin annagrin merged commit d6b26b3 into microsoft:master May 1, 2018
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.

3 participants