Re: Type assertions without GCC builtins

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Type assertions without GCC builtins
Date: 2025-11-19 18:44:20
Message-ID: 0a842ef7-ace3-4868-98b4-0e5f104ed35c@eisentraut.org
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 15.11.25 01:06, Thomas Munro wrote:
> Our assertion quality is lower on Visual Studio. I assumed there was
> nothing stopping us from writing a pg_expr_has_type_p() macro using
> pure standard C and C++ these days, and the attached patch seemed to
> work on GCC, Clang and Visual Studio 2022 (via CI, no Windows here).
>
> Unfortunately our unconstify() macro triggers internal errors on
> Visual Studio 2019 with this applied:
>
> [23:39:47.176] ../src/common/file_utils.c(712): fatal error C1001:
> Internal compiler error.
> [23:39:47.176] (compiler file 'msc1.cpp', line 1603)
> [23:39:47.176] To work around this problem, try simplifying or
> changing the program near the locations listed above.
>
> Presumably _Generic type resolution and StaticAssertExpr()'s
> definition are just too much for it. I wonder if some other phrasing
> could help. Posting where I got to with this, in case anyone has any
> ideas...

Yeah, I had been playing with a similar patch, which now also crashes on
CI, but I'm pretty sure this worked at some point. So maybe an upgrade
or downgrade would fix it.

Note however that similar to the comment I made over about stdatomic.h,
we currently still have buildfarm members with gcc <4.9, which don't
support _Generic.

The attached 0001 patch, which is also contained in your patch, should
be applied nonetheless. After researching the history, I think when
relptr.h was added, it just took a shortcut with the configure checks
available at the time, and we should just correct it now.

I did not consider C++. I'm unsure what to do about it. For the C type
system, "compatible" is term of art, and swapping
__builtin_types_compatible_p for _Generic is semantically equivalent. I
don't have the C++ experience to know what exactly std::is_same is, but
I don't know that we want to expose ourselves to requiring types to be
*both* C-"compatible" and C++-"same" without more guidance.

In your patch, you also rewrite the unconstify() and unvolatize()
macros. At least in my head, these were initially modeled as C versions
of C++ const_cast. So it seems morally imperative to keep that
association. ;-)

I don't know what the aim of the C++ support might be. Is there a
chance that with the right set of changes, for example, "lib/ilist.h" or
"lib/pairingheap.h" could be used under C++?

Attachment Content-Type Size
vPE-0001-Use-correct-preprocessor-conditional-in-relptr.h.patch text/plain 2.2 KB
vPE-0002-Replace-__builtin_types_compatible_p-with-_Gener.patch text/plain 7.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Paul A Jungwirth 2025-11-19 18:49:59 Re: SQL:2011 Application Time Update & Delete
Previous Message Antonin Houska 2025-11-19 18:42:26 Avoid unnecessary processing of DISTINCT clause (Was: Unique Keys)