Обсуждение: anonymous unions (C11)

Here is another patch set to sprinkle some C11 features around the
code.  My aim is to make a little bit of use of several C11 features
as examples and encouragement for future code, and to test compilers
(although we're not yet at the point where this should really stress
any compiler).

Here, I'm proposing to make some use of anonymous unions, which are a
nice notational simplification if you nest unions inside structs,
which is pretty common in PostgreSQL code.

Often times, you have something like

struct important_thing
{
    int a;
    int b;
    union
    {
        int foo;
        int bar;
    } u;
}

(in PostgreSQL source code, often "u" is used, sometimes "d"), and
then you have to address foo as varname.u.foo, which often looks
cryptic (especially if there is more nesting).  With anonymous unions,
you can declare this as

struct important_thing
{
    int a;
    int b;
    union
    {
        int foo;
        int bar;
    };
}

and write varname.foo, which often seems clearer.

(C++ also allows this, so this will preserve C++ compatibility of the
headers.)

That said, I did go overboard here and converted all the struct/union
combinations I could find, but I'm not necessarily proposing to apply
all of them.  I'm proposing patches 0001 through 0004, which are
relatively simple or in areas that have already changed a few times
recently (so backpatching would not be trivial anyway), and/or they
are somewhat close to my heart because they originally motivated this
work a long time ago.  But if someone finds among the other patches
one that they particularly like, we could add that one as well.

On Tue, Sep 23, 2025, at 11:38, Peter Eisentraut wrote:
> Here is another patch set to sprinkle some C11 features around the
> code.  My aim is to make a little bit of use of several C11 features
> as examples and encouragement for future code, and to test compilers
> (although we're not yet at the point where this should really stress
> any compiler).

Nice, didn't know about this C11 feature.

I've eyeballed the entire output of `git diff --word-diff-regex=.` and can't see any oddities.

I've also run the full test suite and it passes.

/Joel

On Tue, Sep 23, 2025 at 11:38:22AM +0200, Peter Eisentraut wrote:
> That said, I did go overboard here and converted all the struct/union
> combinations I could find, but I'm not necessarily proposing to apply
> all of them.  I'm proposing patches 0001 through 0004, which are
> relatively simple or in areas that have already changed a few times
> recently (so backpatching would not be trivial anyway), and/or they
> are somewhat close to my heart because they originally motivated this
> work a long time ago.  But if someone finds among the other patches
> one that they particularly like, we could add that one as well.

I would have used this in the DSM registry if it was available.  Patch
attached.

-- 
nathan

On Wed, Sep 24, 2025 at 4:49 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
> On Tue, Sep 23, 2025 at 5:38 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>>
>>
>>
>> That said, I did go overboard here and converted all the struct/union
>> combinations I could find, but I'm not necessarily proposing to apply
>> all of them.  I'm proposing patches 0001 through 0004, which are
>> relatively simple or in areas that have already changed a few times
>> recently (so backpatching would not be trivial anyway), and/or they
>> are somewhat close to my heart because they originally motivated this
>> work a long time ago.  But if someone finds among the other patches
>> one that they particularly like, we could add that one as well.
>
>
> I went through all commits and code changes are neat and clear.
>
> But when I tried to build on my Macbook M4, it got 18 errors against cryptohash.c:
> ```
> cryptohash.c:108:22: error: no member named 'data' in 'struct pg_cryptohash_ctx'
>   108 |                         pg_md5_init(&ctx->data.md5);
>       |                                      ~~~  ^
... snip ...
> cryptohash.c:225:26: error: no member named 'data' in 'struct pg_cryptohash_ctx'
>   225 |                         pg_sha512_final(&ctx->data.sha512, dest);
>       |                                          ~~~  ^
> 18 errors generated.

I am using meson and gcc with following versions

$ gcc --version
gcc (Ubuntu 11.4.0-1ubuntu1~22.04.2) 11.4.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ meson --version
0.61.2

I am getting opposite of those errors

../../coderoot/pg/src/common/cryptohash.c: In function ‘pg_cryptohash_init’:
../../coderoot/pg/src/common/cryptohash.c:108:41: error:
‘pg_cryptohash_ctx’ has no member named ‘md5’
  108 |                         pg_md5_init(&ctx->md5);
      |                                         ^~
... snip ...
../../coderoot/pg/src/common/cryptohash.c:225:45: error:
‘pg_cryptohash_ctx’ has no member named ‘sha512’
  225 |                         pg_sha512_final(&ctx->sha512, dest);

Attached patch fixes those errors for me.

--
Best Wishes,
Ashutosh Bapat

On 2025-Sep-30, Ashutosh Bapat wrote:

> Attached patch fixes those errors for me.

> diff --git a/src/common/cryptohash.c b/src/common/cryptohash.c
> index fc0555d2f99..e8a35c2ec35 100644
> --- a/src/common/cryptohash.c
> +++ b/src/common/cryptohash.c
> @@ -61,7 +61,7 @@ struct pg_cryptohash_ctx
>          pg_sha256_ctx sha256;
>          pg_sha384_ctx sha384;
>          pg_sha512_ctx sha512;
> -    }            data;
> +    };
>  };

Isn't this patch 0015 in Peter's series?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Saca el libro que tu religión considere como el indicado para encontrar la
oración que traiga paz a tu alma. Luego rebootea el computador
y ve si funciona" (Carlos Duclós)

On Tue, Sep 30, 2025 at 5:14 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Sep-30, Ashutosh Bapat wrote:
>
> > Attached patch fixes those errors for me.
>
> > diff --git a/src/common/cryptohash.c b/src/common/cryptohash.c
> > index fc0555d2f99..e8a35c2ec35 100644
> > --- a/src/common/cryptohash.c
> > +++ b/src/common/cryptohash.c
> > @@ -61,7 +61,7 @@ struct pg_cryptohash_ctx
> >               pg_sha256_ctx sha256;
> >               pg_sha384_ctx sha384;
> >               pg_sha512_ctx sha512;
> > -     }                       data;
> > +     };
> >  };
>
> Isn't this patch 0015 in Peter's series?

I found the compilation errors after pulling the latest sources at
that time. I thought the patch series is committed and hence this
report and patch. Didn't look at the patch series.

But the problematic commit has been reverted since then.

--
Best Wishes,
Ashutosh Bapat

On 2025-Sep-30, Ashutosh Bapat wrote:

> I found the compilation errors after pulling the latest sources at
> that time. I thought the patch series is committed and hence this
> report and patch. Didn't look at the patch series.

Ah, okay.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/