On 12.11.25 15:27, Chao Li wrote:
> 0002 simplifies several structures/unions by using alignas, I have a couple of minor comment:
>
> 1 - 0002
> ```
> -typedef union PGAlignedBlock
> +typedef struct PGAlignedBlock
> {
> - char data[BLCKSZ];
> - double force_align_d;
> - int64 force_align_i64;
> + alignas(MAXIMUM_ALIGNOF) char data[BLCKSZ];
> } PGAlignedBlock;
> ```
>
> As we changes PGAlignedBlock from union to structure, I think we can explicitly mention in the commit message
somethinglike “PGAlignedBlock has the same alignment and contiguous array data, thus no ABI change”.
We don't care about ABI changes in major releases.
> 2 - 0002
> ```
> - /* page_buffer must be adequately aligned, so use a union */
> - union
> - {
> - char buf[QUEUE_PAGESIZE];
> - AsyncQueueEntry align;
> - } page_buffer;
> + /* page_buffer must be adequately aligned */
> + alignas(AsyncQueueEntry) char page_buffer[QUEUE_PAGESIZE];
> ```
>
> To make readers easier to understand the statement, maybe we can explicitly use alignof:
>
> alignas(alignof(AsyncQueueEntry)) char page_buffer[QUEUE_PAGESIZE];
I don't know. alignas(type) is standard C and seems pretty intuitive to
me. Writing it the long way would be potentially more confusing IMO.
