remove pg_restrict workaround
When in C11 mode, MSVC supports the standard "restrict" keyword, so we
don't need the workaround with using "pg_restrict" instead anymore.
(Just for clarification, restrict is a C99 feature, but MSVC only
accepts it properly in C11 mode.) So I'm proposing to remove that
workaround here, so that code can use the standard restrict keyword
without having to worry about the alternative spelling.
However, "restrict" does not exist in C++, but all supported compilers
support the spelling "__restrict" in C++. Therefore, I'm leaving in
place the Autoconf test and the equivalent Meson business, to maintain
the status quo with respect to C++. I'm updating the comments about
that a bit. (In a C++-free world, we could have plausibly removed the
Autoconf test altogether.)
While making the required adjustments, I found a few pieces of code that
use restrict (previously pg_restrict) in what appears to me to be
nonsensical ways, like
memcpy((char *pg_restrict) (buf->data + buf->len), &ni, sizeof(uint16));
The restrict qualifier is not meaningful in casts, so I'm confused about
why it is used here. I did not find any indications either in the old
discussions leading up to this change or on the wider web that there are
any other interpretations or compiler extensions or perhaps a C++ angle
that would make this meaningful. Also, the generated code appears to be
the same if I remove this. So I'm proposing to remove redundant casts
like this.
Attachments:
0001-Remove-meaninglist-restrict-qualifiers.patchtext/plain; charset=UTF-8; name=0001-Remove-meaninglist-restrict-qualifiers.patchDownload+5-6
0002-Replace-pg_restrict-by-standard-restrict.patchtext/plain; charset=UTF-8; name=0002-Replace-pg_restrict-by-standard-restrict.patchDownload+30-54
Peter Eisentraut <peter@eisentraut.org> writes:
When in C11 mode, MSVC supports the standard "restrict" keyword, so we
don't need the workaround with using "pg_restrict" instead anymore.
(Just for clarification, restrict is a C99 feature, but MSVC only
accepts it properly in C11 mode.) So I'm proposing to remove that
workaround here, so that code can use the standard restrict keyword
without having to worry about the alternative spelling.
Won't this break extensions that are using pg_restrict? Sure, they
could update their code, but then maybe it wouldn't work anymore
against previous branches. Seems like it'd be better to leave
pg_restrict in place (for awhile anyway) but always #define it
as "restrict". I don't mind ceasing to use it within our own tree
though.
regards, tom lane
On 15.10.25 15:58, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
When in C11 mode, MSVC supports the standard "restrict" keyword, so we
don't need the workaround with using "pg_restrict" instead anymore.
(Just for clarification, restrict is a C99 feature, but MSVC only
accepts it properly in C11 mode.) So I'm proposing to remove that
workaround here, so that code can use the standard restrict keyword
without having to worry about the alternative spelling.Won't this break extensions that are using pg_restrict? Sure, they
could update their code, but then maybe it wouldn't work anymore
against previous branches. Seems like it'd be better to leave
pg_restrict in place (for awhile anyway) but always #define it
as "restrict". I don't mind ceasing to use it within our own tree
though.
Committed with a backward compatibility define.
On Wed, 29 Oct 2025 at 08:04, Peter Eisentraut <peter@eisentraut.org> wrote:
Committed with a backward compatibility define.
I'm working on adding a C++ extension module to my copyObject
patchset[1]/messages/by-id/CAGECzQR21OnnKiZO_1rLWO0-16kg1JBxnVq-wymYW0-_1cUNtg@mail.gmail.com. But turns out that this change has completely broken
compiling C++ extensions on MSVC. This is happening due to
__declspec(restrict) being replaced by __declspec(__restrict), which
the original comments also mentioned as the reason for having our own
flavor. This replacement then makes the core of a corecrt_malloc.h
MSVC header invalid[2]https://cirrus-ci.com/task/5516067944005632?logs=build#L2031-L2044.
So I think we should revert this patch.
[1]: /messages/by-id/CAGECzQR21OnnKiZO_1rLWO0-16kg1JBxnVq-wymYW0-_1cUNtg@mail.gmail.com
[2]: https://cirrus-ci.com/task/5516067944005632?logs=build#L2031-L2044
Hi,
On 2026-01-03 00:23:37 +0100, Jelte Fennema-Nio wrote:
On Wed, 29 Oct 2025 at 08:04, Peter Eisentraut <peter@eisentraut.org> wrote:
Committed with a backward compatibility define.
I'm working on adding a C++ extension module to my copyObject
patchset[1]. But turns out that this change has completely broken
compiling C++ extensions on MSVC. This is happening due to
__declspec(restrict) being replaced by __declspec(__restrict), which
the original comments also mentioned as the reason for having our own
flavor. This replacement then makes the core of a corecrt_malloc.h
MSVC header invalid[2].So I think we should revert this patch.
+1
Greetings,
Andres Freund
On 03.01.26 00:23, Jelte Fennema-Nio wrote:
On Wed, 29 Oct 2025 at 08:04, Peter Eisentraut <peter@eisentraut.org> wrote:
Committed with a backward compatibility define.
I'm working on adding a C++ extension module to my copyObject
patchset[1]. But turns out that this change has completely broken
compiling C++ extensions on MSVC. This is happening due to
__declspec(restrict) being replaced by __declspec(__restrict), which
the original comments also mentioned as the reason for having our own
flavor. This replacement then makes the core of a corecrt_malloc.h
MSVC header invalid[2].
meson.build already contains a hint about the solution:
# Even though restrict is in C99 and should be supported by all
# supported compilers, this indirection is useful because __restrict
# also works in C++ in all supported compilers. (If not, then we
# might have to write a real test.) (restrict is not part of the C++
# standard.)
cdata.set('restrict', '__restrict')
Or maybe instead of writing a test, we should add something like this to
c.h:
#ifdef __cplusplus
#ifdef __GNUC__
#define restrict __restrict
#else
#define restrict
#endif
#endif
On Sat, 3 Jan 2026 at 18:14, Peter Eisentraut <peter@eisentraut.org> wrote:
meson.build already contains a hint about the solution:
The comment that was there before f0f2c0c1a was better imo, because it
explained the exact problem I ran into with the new definition:
# MSVC doesn't cope well with defining restrict to __restrict, the spelling it
# understands, because it conflicts with __declspec(restrict). Therefore we
# define pg_restrict to the appropriate definition, which presumably won't
# conflict.
#
# We assume C99 support, so we don't need to make this conditional.
cdata.set('pg_restrict', '__restrict')
Or maybe instead of writing a test, we should add something like this to
c.h:#ifdef __cplusplus
#ifdef __GNUC__
#define restrict __restrict
#else
#define restrict
#endif
#endif
I don't think a test or such a define can solve this problem. The
problem is that restrict already has a meaning in MSVC C++. But it's a
different one from C restrict. It's one of the allowed arguments to
__declspec(<arg>). So if we define restrict to anything (whether
__restrict, <empty string>, or foobar) that will always cause issues
in MSVC C++, because then any usage of __declspec(restrict) in the
MSVC stdlib will be replaced with
__declspec(__restrict)/__declspec()/__declspec(foobar).
So for MSVC C++ we cannot do:
#define restrict <whatever>
Which then means that if we continue to declare functions with
restrict in for arguments in headers, then we cannot use any of those
headers in MSVC C++. Because restrict would not be valid in that
position.
I don't see any way out except a revert. To be clear, that would solve
it because we can totally do the following on MSVC C++:
#define pg_restrict __restrict
On 03.01.26 23:57, Jelte Fennema-Nio wrote:
I don't see any way out except a revert. To be clear, that would solve
it because we can totally do the following on MSVC C++:
#define pg_restrict __restrict
Yes, it looks like there is no way past that. I have reverted the patch
and updated the code comments to clarify that the reason for this is now
C++ only.