C nitpick about pgwin32_dispatch_queued_signals()

On 11/2/2025 7:05 AM, Christian Ullrich wrote:

Hello,

the current MSVC compiler deems it necessary to issue

warning C4053: one void operand for '?:'

for a line with CHECK_FOR_INTERRUPTS(). This boils down to this bit of
miscadmin.h (line 116 in master):

#define INTERRUPTS_PENDING_CONDITION() \
(unlikely(UNBLOCKED_SIGNAL_QUEUE()) ?
pgwin32_dispatch_queued_signals() : 0, \
unlikely(InterruptPending))
#endif

The C spec says that of the possible results of the :? operator, either
none or both can be void, and pgwin32_dispatch_queued_signals() is void
(and has been as far back as I can find it).

Does that matter?

Yeah, this is a bug, or at least a spec violation. We should fix it in
my opinion-- it's non-conforming C. Others may disagree, though.

It happens to work because the comma operator discards the result
anyway, but MSVC is right to complain.

This isn't a new thing with modern C standards, BTW --- the rule
about not mixing void and non-void operands in ?: has been there
since C89. We've just been getting away with it because most
compilers don't complain when the result is discarded by the
comma operator anyway.

I see a couple of ways to fix it:

1. Cast both arms to void:

#define INTERRUPTS_PENDING_CONDITION() \
(unlikely(UNBLOCKED_SIGNAL_QUEUE()) ? \
(pgwin32_dispatch_queued_signals(), (void)0) : (void)0, \
unlikely(InterruptPending))

2. Restructure to use && instead of ?::

#define INTERRUPTS_PENDING_CONDITION() \
(unlikely(UNBLOCKED_SIGNAL_QUEUE()) && \
(pgwin32_dispatch_queued_signals(), true), \
unlikely(InterruptPending))

3. Just make it an inline function instead of a macro.

I'd lean towards #2 --- it's cleaner and avoids the whole issue.
The semantics are the same since we're only calling the function
for its side-effects.

Thoughts?

Bryan Green

Bryan Green <dbryan.green@gmail.com> writes:

On 11/2/2025 7:05 AM, Christian Ullrich wrote:

the current MSVC compiler deems it necessary to issue
warning C4053: one void operand for '?:'
for a line with CHECK_FOR_INTERRUPTS(). This boils down to this bit of
miscadmin.h (line 116 in master):

#define INTERRUPTS_PENDING_CONDITION() \
(unlikely(UNBLOCKED_SIGNAL_QUEUE()) ?
pgwin32_dispatch_queued_signals() : 0, \
unlikely(InterruptPending))
#endif

The C spec says that of the possible results of the :? operator, either
none or both can be void, and pgwin32_dispatch_queued_signals() is void
(and has been as far back as I can find it).

Yeah, this is a bug, or at least a spec violation. We should fix it in
my opinion-- it's non-conforming C. Others may disagree, though.

Agreed.

2. Restructure to use && instead of ?::

#define INTERRUPTS_PENDING_CONDITION() \
(unlikely(UNBLOCKED_SIGNAL_QUEUE()) && \
(pgwin32_dispatch_queued_signals(), true), \
unlikely(InterruptPending))

I'd lean towards #2 --- it's cleaner and avoids the whole issue.

I dunno, don't really like the nested comma operator here.
It's probably necessary to avoid having a void argument to &&,
but it's confusing. Let's go with your #1 (casting the 0 to void).
But can't we simplify that to just

#define INTERRUPTS_PENDING_CONDITION() \
(unlikely(UNBLOCKED_SIGNAL_QUEUE()) ?
pgwin32_dispatch_queued_signals() : (void) 0, \
unlikely(InterruptPending))
#endif

that is, the only change needed is s/0/(void) 0/.

regards, tom lane

On Sun, 2025-11-02 at 11:47 -0500, Tom Lane wrote:

Bryan Green <dbryan.green@gmail.com> writes:

On 11/2/2025 7:05 AM, Christian Ullrich wrote:

the current MSVC compiler deems it necessary to issue
warning C4053: one void operand for '?:'
for a line with CHECK_FOR_INTERRUPTS(). This boils down to this bit of
miscadmin.h (line 116 in master):

#define INTERRUPTS_PENDING_CONDITION() \
(unlikely(UNBLOCKED_SIGNAL_QUEUE()) ?
pgwin32_dispatch_queued_signals() : 0, \
unlikely(InterruptPending))
#endif

The C spec says that of the possible results of the :? operator, either
none or both can be void, and pgwin32_dispatch_queued_signals() is void
(and has been as far back as I can find it).

Yeah, this is a bug, or at least a spec violation. We should fix it in
my opinion-- it's non-conforming C. Others may disagree, though.

Agreed.

Let's go with your #1 (casting the 0 to void).
But can't we simplify that to just

#define INTERRUPTS_PENDING_CONDITION() \
(unlikely(UNBLOCKED_SIGNAL_QUEUE()) ?
pgwin32_dispatch_queued_signals() : (void) 0, \
unlikely(InterruptPending))
#endif

that is, the only change needed is s/0/(void) 0/.

+1

Laurenz Albe

On 11/2/2025 10:50 AM, Laurenz Albe wrote:

On Sun, 2025-11-02 at 11:47 -0500, Tom Lane wrote:

Bryan Green <dbryan.green@gmail.com> writes:

On 11/2/2025 7:05 AM, Christian Ullrich wrote:

the current MSVC compiler deems it necessary to issue
warning C4053: one void operand for '?:'
for a line with CHECK_FOR_INTERRUPTS(). This boils down to this bit of
miscadmin.h (line 116 in master):

#define INTERRUPTS_PENDING_CONDITION() \
(unlikely(UNBLOCKED_SIGNAL_QUEUE()) ?
pgwin32_dispatch_queued_signals() : 0, \
unlikely(InterruptPending))
#endif

The C spec says that of the possible results of the :? operator, either
none or both can be void, and pgwin32_dispatch_queued_signals() is void
(and has been as far back as I can find it).

Yeah, this is a bug, or at least a spec violation. We should fix it in
my opinion-- it's non-conforming C. Others may disagree, though.

Agreed.

Let's go with your #1 (casting the 0 to void).
But can't we simplify that to just

#define INTERRUPTS_PENDING_CONDITION() \
(unlikely(UNBLOCKED_SIGNAL_QUEUE()) ?
pgwin32_dispatch_queued_signals() : (void) 0, \
unlikely(InterruptPending))
#endif

that is, the only change needed is s/0/(void) 0/.

+1

Laurenz Albe

Option 1 works for me as well. Hopefully, it works for Christian as well.