| From: | "Jelte Fennema-Nio" <postgres(at)jeltef(dot)nl> |
|---|---|
| To: | "Andres Freund" <andres(at)anarazel(dot)de> |
| Cc: | "Peter Eisentraut" <peter(at)eisentraut(dot)org>, "PostgreSQL Hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Thomas Munro" <thomas(dot)munro(at)gmail(dot)com> |
| Subject: | Re: Make copyObject work in C++ |
| Date: | 2026-01-25 17:52:37 |
| Message-ID: | DFXV193BOBQ8.1BY2PK8YRJ9O1@jeltef.nl |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun Jan 25, 2026 at 5:50 PM CET, Andres Freund wrote:
> I'm pretty sceptical this is the right direction.
The only other option I can think of is not supporting C++ extension on
MSVC unless they are compiled with C++20 (or later). I don't
particularly care about Windows C++ extensions myself, so I personally
would be fine with that choice. It seems a bit harsh, though.
> We were going for designated
> initializers for a reason, namely that we expect more arguments to be added
> over time and perhaps eventually also to remove some. And this will just lead
> to that being harder because we have to worry about C++ extensions.
Adding new arguments (aka fields) should cause no problems. Assuming
we'd add them at the end of the Pg_magic_struct definition. Removing
ones seems like even for C you'd need different PG_MODULE_MAGIC_EXT
invocations depending on PG_VERSION_NUM. I don't see how using
positional args would make that harder.
> But I'm also confused as to why it's needed - there's nomixing of designated
> and non-designated initializers that I can see? If you use
> PG_MODULE_MAGIC_EXT(.name = "whatnot"), it evaluates down to
>
> extern __attribute__((visibility("default"))) const Pg_magic_struct *Pg_magic_func(void); const Pg_magic_struct * Pg_magic_func(void) { static const Pg_magic_struct Pg_magic_data = { .len = sizeof(Pg_magic_struct), .abi_fields = { 190000 / 100, 100, 32, 64, true, "PostgreSQL", }, .name="whatnot"}; return &Pg_magic_data; } extern int no_such_variable;
>
> And indeed, contra to what you reported upthread, I can't get clang to report
> a warning about that. I do obviously see warnings about the wrong order if I
> pass the arguments in the wrong order, but that's a lot less problematic. And
> omitted args don't trigger warnings, as you noted.
It sounds like I wasn't clear enough what the problem was. The main
problem currently is that MSVC C++ fails to compile a cpp file
containing PG_MODULE_MAGIC (or PG_MODULE_MAGIC_EXT) unless you use /std:c++20.
Afaict it would have been possible to do so before 9324c8c580 (aka
anything before PG18), but since that commit you need /std:c++20. The
fact that we haven't heard anyone complain so far, might be an indication
that not many people (or maybe anyone) is using C++ extensions on Windows.
The only way to make PG_MODULE_MAGIC work on MSVC pre-C++20 is to
partially revert 9324c8c580, and use positional arguments in the
definition of PG_MODULE_MAGIC_DATA again. Like I've done in v7-0001.
For C extensions v7-0001 has no downsides. However, after applying
v7-0001, C++ extensions that use PG_MODULE_MAGIC_EXT with designated
parameters will get the following warning on at least clang 18 on my
machine:
meson setup --prefix ~/.pgenv/pgsql-master --debug build --reconfigure -Dc_args='-fno-omit-frame-pointer' -Dcassert=true
meson test -C build --suite setup --suite test_cplusplusext
[2202/2268] Compiling C++ object src/test/modules/test_cplusplusext/test_cplusplusext.so.p/test_cplusplusext.cpp.o
../src/test/modules/test_cplusplusext/test_cplusplusext.cpp:23:21: warning: mixture of designated and non-designated initializers in the same initializer list is a C99 extension [-Wc99-designator]
23 | PG_MODULE_MAGIC_EXT(.name="test_cplusplusext",.version= "1.2");
| ^~~~~~~~~~~~~~~~~~~~~~~~~
../src/include/fmgr.h:548:24: note: expanded from macro 'PG_MODULE_MAGIC_EXT'
548 | PG_MODULE_MAGIC_DATA(__VA_ARGS__); \
| ^~~~~~~~~~~
../src/include/fmgr.h:509:2: note: expanded from macro 'PG_MODULE_MAGIC_DATA'
509 | __VA_ARGS__ \
| ^~~~~~~~~~~
../src/test/modules/test_cplusplusext/test_cplusplusext.cpp:23:1: note: first non-designated initializer is here
23 | PG_MODULE_MAGIC_EXT(.name="test_cplusplusext",.version= "1.2");
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/include/fmgr.h:548:3: note: expanded from macro 'PG_MODULE_MAGIC_EXT'
548 | PG_MODULE_MAGIC_DATA(__VA_ARGS__); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/include/fmgr.h:507:2: note: expanded from macro 'PG_MODULE_MAGIC_DATA'
507 | sizeof(Pg_magic_struct), \
| ^~~~~~~~~~~~~~~~~~~~~~~
Note that you need to use meson test, not meson install, otherwise
test_cplusplusext.cpp won't get compiled.
So to be clear, yes right now there's no mixing of designated and
non-designated initializers. But after applying v7-0001 there would be
when you use something like this:
PG_MODULE_MAGIC_EXT(.name="test_cplusplusext",.version= "1.2");
And while the C standard allows mixing designated and non-designated
initializerss, the C++ standard (even C++20) does not.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Mihail Nikalayeu | 2026-01-25 18:39:00 | Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY |
| Previous Message | Sami Imseih | 2026-01-25 17:23:39 | Re: Optional skipping of unchanged relations during ANALYZE? |