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

[intro.object] Make the storage in the example for storage providing properly aligned #6422

Merged
merged 2 commits into from
Aug 30, 2023

Conversation

frederick-vs-ja
Copy link
Contributor

Fixes cplusplus/CWG#379.

The example seemingly assumes that the implementation-defined properties on common platforms (e.g., sizeof(int) is 4) hold. I guess we can keep such assumptions implicit and avoid pedantically writing sizeof(int).

source/basic.tex Show resolved Hide resolved
source/basic.tex Show resolved Hide resolved
@JohelEGP
Copy link
Contributor

Something like this has come up recently.
We could just use types like std::int32_t instead in these examples.
Then the examples clearly work, and clearly depend on those types being present.
Is anyone's stance known to be against that?

@CaseyCarter
Copy link
Contributor

CaseyCarter commented Jul 28, 2023

Something like this has come up recently. We could just use types like std::int32_t instead in these examples. Then the examples clearly work, and clearly depend on those types being present. Is anyone's stance known to be against that?

I'm not opposed to int32_t, but I will point out that there's no guarantee that sizeof(int32_t) > 1.

@JohelEGP
Copy link
Contributor

So this kind of example only needs an "assumes CHAR_BIT is 8" clause, right?

@CaseyCarter
Copy link
Contributor

So this kind of example only needs an "assumes CHAR_BIT is 8" clause, right?

Or equivalently, static_assert(sizeof(std::int32_t) == 4) which simultaneously requires int32_t to exist and CHAR_BIT to be 8.

@JohelEGP
Copy link
Contributor

Well, examples would require #include <cstdint>.
Maybe the first of these examples
should describe the parameters to the instance of the abstract machine they work on
and have the other similar examples refer to that.

@JohelEGP
Copy link
Contributor

So adding

+#include <cstdint>
+static_assert(sizeof(std::int32_t) == 4);

and changing the integer type keywords to the appropriate std:: type,
vs.

+// Assume the abstract machine described in \ref{that.first.example}.

for the rest of the examples.

Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

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

Looks like a good, minimal fix to me.

@xmh0511
Copy link
Contributor

xmh0511 commented Jul 30, 2023

Note [basic.align] p1:

An alignment is an implementation-defined integer value representing the number of bytes between successive addresses at which a given object can be allocated.

alignas(int) A a; can only guarantee the address (named A) of a satisfy the alignment of int, however, in the fix, we intend to use A + n * sizeof(int) to prove the result addresses satisfy the alignment, note that this is true only if sizeof(int) equal to alignof(int) because [basic.align] p1 only mention the alignment value, anyway, sizeof(int) and alignof(int) are both implementation-defined values that are not necessary the same.

@frederick-vs-ja
Copy link
Contributor Author

anyway, sizeof(int) and alignof(int) are both implementation-defined values that are not necessary the same.

This is OK. Assuming that sizeof(int) is 4, alignof(int) must be 1, 2, or 4, and each of these values is OK for the example.

@xmh0511
Copy link
Contributor

xmh0511 commented Jul 30, 2023

Assuming that sizeof(int) is 4, alignof(int) must be 1, 2, or 4

However, there is no wording in the current standard that says the value of alignof(int) must be less or equal to sizeof(int), if alignof(int) is 8 and sizeof(int) is 4, it will be an issue.

@jensmaurer
Copy link
Member

However, there is no wording in the current standard that says the value of alignof(int) must be less or equal to sizeof(int), if alignof(int) is 8 and sizeof(int) is 4, it will be an issue.

This is not possible, because in such an implementation, if you have int a[20];, every second int in that array will be misaligned, which can't be.

@tkoeppe tkoeppe merged commit cae5560 into cplusplus:main Aug 30, 2023
2 checks passed
@frederick-vs-ja frederick-vs-ja deleted the align-storage branch August 30, 2023 04:47
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.

[intro.object.example] p1 may violates [basic.align] p1
6 participants