Обсуждение: Type assertions without GCC builtins

Hi,

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...

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++?

On Wed, 19 Nov 2025 at 19:44, Peter Eisentraut <peter@eisentraut.org> wrote:
> 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.

The cases in the "example" section on cppreference.com all seem very
sensible[1]. I'd say if there's some difference with the generic one,
I don't think we'll care about that difference in practice.

> 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. ;-)

You mean use const_cast in c++ instead of a C-style cast? Would you
still want to keep the static asserts.

> 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++?

I think for those headers you mention it shouldn't matter for
functionality, because the functionality added by the _Generic is
optional type checking. But for my proposal for nicer hash
initialization macros it would be a requirement[2].

[1]: https://en.cppreference.com/w/cpp/types/is_same.html
[2]: https://www.postgresql.org/message-id/flat/aS2b3LoUypW1/Gdz%40ip-10-97-1-34.eu-west-3.compute.internal

On Tue, 9 Dec 2025 at 13:43, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> You mean use const_cast in c++ instead of a C-style cast? Would you
> still want to keep the static asserts.

I now understand that you meant to not remove these lines:
-#if defined(__cplusplus)
-#define unconstify(underlying_type, expr) const_cast<underlying_type>(expr)
-#define unvolatize(underlying_type, expr) const_cast<underlying_type>(expr)

So like the attached. I think that makes sense.

On 19.11.25 19:44, Peter Eisentraut wrote:
> 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 have committed this 0001 patch.  The rest of this discussion is still 
pending.