Re: Fixing MSVC's inability to detect elog(ERROR) does not return
David Rowley <dgrowleyml@gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2025-09-17T03:52:54Z
Lists: pgsql-hackers
Attachments
- v5-0001-Detect-elog-ERROR-can-t-return-in-MSVC-when-using.patch (application/octet-stream) patch v5-0001
On Wed, 3 Sept 2025 at 23:32, Peter Eisentraut <peter@eisentraut.org> wrote: > The variant using _Generic is not a full replacement for > __builtin_constant_p(), because it only detects integer constant > expressions. So for example, it wouldn't work in the use in > src/include/utils/memutils.h, which checks for constant strings. So I > think we need to be careful here to maintain the difference. Makes sense. > I think what we could do is make a separate macro for detecting integer > constant expressions (like ICE_P() in the reddit thread) and define that > to __builtin_constant_p if available, else using _Generic. (We can't > use _Generic on all platforms yet, that's a separate undertaking, but I > think all platforms support either __builtin_constant_p or _Generic.) > And then use that one for ereport. Done > It would also be nice to provide a comment with some explanation and/or > a link and credit for the _Generic expression. I've added a link to the stackoverflow thread in the comment above the macro's definition. > Btw., I think we should stick to the *_p() naming (for "predicate", I > think) for compiler-intrinsic-affiliated functions/macros that report > boolean results. I didn't know what the _p suffix was meant to indicate. Do you have a link which states that it's for "predicate"? The other things I had in mind were "pointer" and "proprocessor", neither of which makes much sense to me. Looking through [1], the only other intrinsic function I see ending in _p is __builtin_types_compatible_p(). I don't see what these both have in common with each other. I've modified the patch to include the _p, however, I'd personally rather leave this out as if someone asks me what it's for, I've got no answer for them, other than Peter said. > The __STDC_VERSION__ comparison can be dropped, since that is the > minimum now required. (But you need to keep defined(__STDC_VERSION__), > since this won't work in C++.) Done. Updated patch attached. Thanks for the review. David [1] https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
Commits
-
Rename pg_builtin_integer_constant_p to pg_integer_constant_p
- 8e2acda2b098 19 (unreleased) landed
-
Teach MSVC that elog/ereport ERROR doesn't return
- 59c2f03d1ece 19 (unreleased) landed