pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedpatch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
14+ messages / 6 participants
[nested] [flat]
Commits from this thread:
2025-07-15 2a3a396432 Clarify the ra != rb case in compareJsonbContainers().
2025-07-15 6c93bf735c 0b6dfce0ce 5a2139a906 5db55e13f2 f32e456414 aad1617b76 8254b7e33b Silence uninitialized-value warnings in compareJsonbContainers().
* patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-10 20:00 Dmitry Mityugov <d.mityugov@postgrespro.ru>
0 siblings, 2 replies; 14+ messages in thread
From: Dmitry Mityugov @ 2025-07-10 20:00 UTC (permalink / raw)
To: pgsql-hackers
When compiled with Assert() macro disabled, GCC 15 produces warnings
about possibly uninitialized variables in
src/backend/utils/adt/jsonb_util.c module. This problem was discussed in
detail in this thread, in April 2025:
https://www.postgresql.org/message-id/988bf1bc-3f1f-99f3-bf98-222f1cd9dc5e@xs4all.nl
.
Recently introduced pg_assume() macro let fix such problems easily. The
attached patch fixes them in jsonb_util.c module. I verified that
PostgreSQL compiles clearly with this patch and GCC 15.1.1 on an x86
64-bit machine (with and without --enable-cassert), and with GCC 14.2.1
on a 64-bit ARM machine. `make check` also passes.
I'm attaching the patch.
Regards,
Dmitry
Attachments:
0001-Use-pg_assume-in-jsonb_util.c-to-fix-GCC-15-warning.patch (text/x-diff, 1.1K) download
From: Dmitry Mityugov <d.mityugov@postgrespro.ru>
Date: Thu, 10 Jul 2025 22:05:12 +0300
Subject: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
Recently introduced pg_assume macro is a great tool to silence GCC 15 warnings
generated for release builds, when Assert() macro is disabled. This patch fixes
two of them in backend/utils/adt/jsonb_util.c module.
---
src/backend/utils/adt/jsonb_util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index c18bf741521..2478346ce19 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -291,8 +291,8 @@ compareJsonbContainers(JsonbContainer *a, JsonbContainer *b)
* WJB_BEGIN_ARRAY and WJB_BEGIN_OBJECT tokens first, and
* concluded that they don't match.
*/
- Assert(ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
- Assert(rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);
+ pg_assume(ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
+ pg_assume(rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);
Assert(va.type != vb.type);
Assert(va.type != jbvBinary);
--
2.50.1
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-11 10:20 Erik Rijkers <er@xs4all.nl>
parent: Dmitry Mityugov <d.mityugov@postgrespro.ru>
1 sibling, 0 replies; 14+ messages in thread
From: Erik Rijkers @ 2025-07-11 10:20 UTC (permalink / raw)
To: Dmitry Mityugov <d.mityugov@postgrespro.ru>; pgsql-hackers
Op 7/10/25 om 22:00 schreef Dmitry Mityugov:
> When compiled with Assert() macro disabled, GCC 15 produces warnings
> about possibly uninitialized variables in
> src/backend/utils/adt/jsonb_util.c module. This problem was discussed in
> detail in this thread, in April 2025:
> https://www.postgresql.org/message-id/988bf1bc-3f1f-99f3-bf98-222f1cd9dc5e@xs4all.nl .
>
> Recently introduced pg_assume() macro let fix such problems easily. The
> attached patch fixes them in jsonb_util.c module. I verified that
> PostgreSQL compiles clearly with this patch and GCC 15.1.1 on an x86
> 64-bit machine (with and without --enable-cassert), and with GCC 14.2.1
> on a 64-bit ARM machine. `make check` also passes.
Yes, compiles fine for me with gcc 15.1.0 (with and without
--enable-cassert). It's nice to finally get a silent compile again.
Thanks,
Erik
>
> I'm attaching the patch.
>
> Regards,
> Dmitry
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-12 17:42 Tom Lane <tgl@sss.pgh.pa.us>
parent: Dmitry Mityugov <d.mityugov@postgrespro.ru>
1 sibling, 1 reply; 14+ messages in thread
From: Tom Lane @ 2025-07-12 17:42 UTC (permalink / raw)
To: Dmitry Mityugov <d.mityugov@postgrespro.ru>; +Cc: pgsql-hackers
Dmitry Mityugov <d.mityugov@postgrespro.ru> writes:
> When compiled with Assert() macro disabled, GCC 15 produces warnings
> about possibly uninitialized variables in
> src/backend/utils/adt/jsonb_util.c module. This problem was discussed in
> detail in this thread, in April 2025:
> https://www.postgresql.org/message-id/988bf1bc-3f1f-99f3-bf98-222f1cd9dc5e@xs4all.nl
> .
> Recently introduced pg_assume() macro let fix such problems easily. The
> attached patch fixes them in jsonb_util.c module. I verified that
> PostgreSQL compiles clearly with this patch and GCC 15.1.1 on an x86
> 64-bit machine (with and without --enable-cassert), and with GCC 14.2.1
> on a 64-bit ARM machine. `make check` also passes.
I don't care for this patch: replacing an Assert with pg_assume just
seems like a very bad idea. If the assertion condition ever failed,
which doesn't seem all that impossible in logic as complicated as
this, then instead of an identifiable assertion trap we'd get
unspecified and impossible-to-debug behavior.
We could conceivably do
+#ifdef USE_ASSERT_CHECKING
Assert(ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
Assert(rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);
+#else
+ pg_assume(ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
+ pg_assume(rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);
+#endif
but that seems ugly. In any case, it's just doubling down on the
assumption that a compiler capable of detecting the "may be used
uninitialized" issue will conclude that rejecting WJB_END_ARRAY
and WJB_END_OBJECT (but not WJB_DONE) eliminates the possibility of
va.type/vb.type not being set.
What I think we should do about this is what I mentioned in the
other thread: adjust the code to guarantee that va.type/vb.type
are defined in all cases. There are a couple of ways we could
do that, but after reflection I think the best way is to modify
JsonbIteratorNext to make that guarantee. I've checked that
the attached silences the warning on gcc 15.1.1 (current
Fedora 42).
regards, tom lane
Attachments:
v2-silence-uninitialized-warnings.patch (text/x-diff, 2.1K) download
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index c8b6c15e059..136952861e1 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -277,9 +277,6 @@ compareJsonbContainers(JsonbContainer *a, JsonbContainer *b)
else
{
/*
- * It's safe to assume that the types differed, and that the va
- * and vb values passed were set.
- *
* If the two values were of the same container type, then there'd
* have been a chance to observe the variation in the number of
* elements/pairs (when processing WJB_BEGIN_OBJECT, say). They're
@@ -852,15 +849,20 @@ JsonbIteratorInit(JsonbContainer *container)
* It is our job to expand the jbvBinary representation without bothering them
* with it. However, clients should not take it upon themselves to touch array
* or Object element/pair buffers, since their element/pair pointers are
- * garbage. Also, *val will not be set when returning WJB_END_ARRAY or
- * WJB_END_OBJECT, on the assumption that it's only useful to access values
- * when recursing in.
+ * garbage.
+ *
+ * *val is not meaningful when the result is WJB_DONE, WJB_END_ARRAY or
+ * WJB_END_OBJECT. However, we set val->type = jbvNull in those cases,
+ * so that callers may assume that val->type is always well-defined.
*/
JsonbIteratorToken
JsonbIteratorNext(JsonbIterator **it, JsonbValue *val, bool skipNested)
{
if (*it == NULL)
+ {
+ val->type = jbvNull;
return WJB_DONE;
+ }
/*
* When stepping into a nested container, we jump back here to start
@@ -898,6 +900,7 @@ recurse:
* nesting).
*/
*it = freeAndGetParent(*it);
+ val->type = jbvNull;
return WJB_END_ARRAY;
}
@@ -951,6 +954,7 @@ recurse:
* of nesting).
*/
*it = freeAndGetParent(*it);
+ val->type = jbvNull;
return WJB_END_OBJECT;
}
else
@@ -995,8 +999,10 @@ recurse:
return WJB_VALUE;
}
- elog(ERROR, "invalid iterator state");
- return -1;
+ elog(ERROR, "invalid jsonb iterator state");
+ /* satisfy compilers that don't know that elog(ERROR) doesn't return */
+ val->type = jbvNull;
+ return WJB_DONE;
}
/*
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-12 17:55 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 14+ messages in thread
From: Andres Freund @ 2025-07-12 17:55 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Dmitry Mityugov <d.mityugov@postgrespro.ru>; pgsql-hackers
Hi,
On 2025-07-12 13:42:54 -0400, Tom Lane wrote:
> Dmitry Mityugov <d.mityugov@postgrespro.ru> writes:
> > When compiled with Assert() macro disabled, GCC 15 produces warnings
> > about possibly uninitialized variables in
> > src/backend/utils/adt/jsonb_util.c module. This problem was discussed in
> > detail in this thread, in April 2025:
> > https://www.postgresql.org/message-id/988bf1bc-3f1f-99f3-bf98-222f1cd9dc5e@xs4all.nl
> > .
>
> > Recently introduced pg_assume() macro let fix such problems easily. The
> > attached patch fixes them in jsonb_util.c module. I verified that
> > PostgreSQL compiles clearly with this patch and GCC 15.1.1 on an x86
> > 64-bit machine (with and without --enable-cassert), and with GCC 14.2.1
> > on a 64-bit ARM machine. `make check` also passes.
>
> I don't care for this patch: replacing an Assert with pg_assume just
> seems like a very bad idea. If the assertion condition ever failed,
> which doesn't seem all that impossible in logic as complicated as
> this, then instead of an identifiable assertion trap we'd get
> unspecified and impossible-to-debug behavior.
That shouldn't be a problem - pg_assume() is defined to be an Assert() in
USE_ASSERT_CHECKING builds.
> We could conceivably do
>
> +#ifdef USE_ASSERT_CHECKING
> Assert(ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
> Assert(rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);
> +#else
> + pg_assume(ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
> + pg_assume(rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);
> +#endif
>
> but that seems ugly. In any case, it's just doubling down on the
> assumption that a compiler capable of detecting the "may be used
> uninitialized" issue will conclude that rejecting WJB_END_ARRAY
> and WJB_END_OBJECT (but not WJB_DONE) eliminates the possibility of
> va.type/vb.type not being set.
I had played with using pg_assume here too, but I couldn't really convince
myself that it's a good idea...
> What I think we should do about this is what I mentioned in the
> other thread: adjust the code to guarantee that va.type/vb.type
> are defined in all cases. There are a couple of ways we could
> do that, but after reflection I think the best way is to modify
> JsonbIteratorNext to make that guarantee. I've checked that
> the attached silences the warning on gcc 15.1.1 (current
> Fedora 42).
WFM.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-12 18:04 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 1 reply; 14+ messages in thread
From: Tom Lane @ 2025-07-12 18:04 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Dmitry Mityugov <d.mityugov@postgrespro.ru>; pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> On 2025-07-12 13:42:54 -0400, Tom Lane wrote:
>> I don't care for this patch: replacing an Assert with pg_assume just
>> seems like a very bad idea.
> That shouldn't be a problem - pg_assume() is defined to be an Assert() in
> USE_ASSERT_CHECKING builds.
Ah, my bad. But there's still the question of exactly what reasoning
the compiler is using to arrive at the conclusion that it need not
warn given these assertions, and whether we want to rely on that
reasoning not changing. I'd prefer to simplify matters.
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-12 19:23 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 14+ messages in thread
From: Tom Lane @ 2025-07-12 19:23 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Dmitry Mityugov <d.mityugov@postgrespro.ru>; pgsql-hackers
After further study I've understood what was bothering me about
the logic in compareJsonbContainers (lines 280ff). Because
JsonbIteratorNext doesn't presently guarantee to set val->type
when returning WJB_DONE, the stanza appears to be at risk of
undefined behavior should one iterator return that while the other
returns something else. The comment fails to discuss this case,
and the code doesn't consider it either. But in reality, the case
cannot happen for the exact same reason that WJB_END_ARRAY and
WJB_END_OBJECT can't occur: our earlier tests of object and array
structure & length matching guarantee that we can't reach the end
of one input before the other.
So I think we need to rewrite that comment and extend the assertions,
along the lines of the separate patch attached. This gets through
check-world, which is unsurprising because if it did not we'd have
seen use-of-undefined-value Valgrind complaints long since.
I still think that we should silence the compiler warnings by
removing the undefined-ness per my previous patch. I'm inclined
to back-patch that (for people compiling old branches with latest
compiler) but apply this bit to HEAD only (since it's just a code
clarification).
regards, tom lane
Attachments:
v3-rationalize-compareJsonbContainers.patch (text/x-diff, 1.7K) download
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index c8b6c15e059..e73906dc09d 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -277,22 +277,16 @@ compareJsonbContainers(JsonbContainer *a, JsonbContainer *b)
else
{
/*
- * It's safe to assume that the types differed, and that the va
- * and vb values passed were set.
- *
- * If the two values were of the same container type, then there'd
- * have been a chance to observe the variation in the number of
- * elements/pairs (when processing WJB_BEGIN_OBJECT, say). They're
- * either two heterogeneously-typed containers, or a container and
- * some scalar type.
- *
- * We don't have to consider the WJB_END_ARRAY and WJB_END_OBJECT
- * cases here, because we would have seen the corresponding
- * WJB_BEGIN_ARRAY and WJB_BEGIN_OBJECT tokens first, and
- * concluded that they don't match.
+ * It's not possible for one iterator to report end of array or
+ * object while the other one reports something else, because we
+ * would have detected a length mismatch when we processed the
+ * container-start tokens above. Likewise we can't see WJB_DONE
+ * from one but not the other. They're either two different-type
+ * containers, or a container and some scalar type, or two
+ * different scalar types. Sort on the basis of the type.
*/
- Assert(ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
- Assert(rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);
+ Assert(ra != WJB_DONE && ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
+ Assert(rb != WJB_DONE && rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);
Assert(va.type != vb.type);
Assert(va.type != jbvBinary);
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-15 22:35 Peter Geoghegan <pg@bowt.ie>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 2 replies; 14+ messages in thread
From: Peter Geoghegan @ 2025-07-15 22:35 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Dmitry Mityugov <d.mityugov@postgrespro.ru>; pgsql-hackers
On Sat, Jul 12, 2025 at 1:55 PM Andres Freund <andres@anarazel.de> wrote:
> I had played with using pg_assume here too, but I couldn't really convince
> myself that it's a good idea...
In the past, it was often necessary to work around MSVC's inability to
see that a block containing elog(ERROR) doesn't actually need to
initialize variables that'll never actually be used in code that comes
after that block. We still have many "keep compiler quiet" variable
initializations due to this.
Is that still something that we need to worry about? I don't recall
running into it in quite a few years, though that might just be a
coincidence.
It looks like MSVC uses __assume for this now, by way of
elog/ereport's use of pg_unreachable. You also used __assume to
implement pg_assume for MSVC. It seems reasonable to surmise that we
can officially stop worrying about elog(ERROR) related warnings/to
suppose that we no longer have to add "keep compiler quiet" variable
initializations after an elog(ERROR). If it works for MSVC +
pg_assume, then it ought to also work for MSVC + pg_unreachable.
Right?
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-15 22:58 Tom Lane <tgl@sss.pgh.pa.us>
parent: Peter Geoghegan <pg@bowt.ie>
1 sibling, 1 reply; 14+ messages in thread
From: Tom Lane @ 2025-07-15 22:58 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Andres Freund <andres@anarazel.de>; Dmitry Mityugov <d.mityugov@postgrespro.ru>; pgsql-hackers
Peter Geoghegan <pg@bowt.ie> writes:
> In the past, it was often necessary to work around MSVC's inability to
> see that a block containing elog(ERROR) doesn't actually need to
> initialize variables that'll never actually be used in code that comes
> after that block. We still have many "keep compiler quiet" variable
> initializations due to this.
> Is that still something that we need to worry about?
Good question. It's not very clear what set of compiler versions
people are still using, but maybe it'd be okay to stop worrying
about suppressing such warnings.
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-15 23:05 Peter Geoghegan <pg@bowt.ie>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 14+ messages in thread
From: Peter Geoghegan @ 2025-07-15 23:05 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; Dmitry Mityugov <d.mityugov@postgrespro.ru>; pgsql-hackers
On Tue, Jul 15, 2025 at 6:58 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Good question. It's not very clear what set of compiler versions
> people are still using, but maybe it'd be okay to stop worrying
> about suppressing such warnings.
It would be nice to eliminate all existing "keep compiler quiet"
variable initializations that follow an elog/ereport with elevel >=
ERROR. My guess is that we have several hundred.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-15 23:27 Tom Lane <tgl@sss.pgh.pa.us>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 14+ messages in thread
From: Tom Lane @ 2025-07-15 23:27 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Andres Freund <andres@anarazel.de>; Dmitry Mityugov <d.mityugov@postgrespro.ru>; pgsql-hackers
Peter Geoghegan <pg@bowt.ie> writes:
> It would be nice to eliminate all existing "keep compiler quiet"
> variable initializations that follow an elog/ereport with elevel >=
> ERROR. My guess is that we have several hundred.
There are a lot of them, for sure. I'd be a bit worried about
creating a back-patching mine-field. But maybe these are all
in spots we're unlikely to touch?
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-15 23:30 Peter Geoghegan <pg@bowt.ie>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 14+ messages in thread
From: Peter Geoghegan @ 2025-07-15 23:30 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; Dmitry Mityugov <d.mityugov@postgrespro.ru>; pgsql-hackers
On Tue, Jul 15, 2025 at 7:27 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> There are a lot of them, for sure.
It's *very* common for switch statements to have a "can't happen"
elog(ERROR) as their default block. Many of these default blocks also
initialize related variables to placate the compiler.
> I'd be a bit worried about
> creating a back-patching mine-field. But maybe these are all
> in spots we're unlikely to touch?
That seems like much less of a problem for a purely subtractive change
such as this.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-15 23:52 Tom Lane <tgl@sss.pgh.pa.us>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 14+ messages in thread
From: Tom Lane @ 2025-07-15 23:52 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Andres Freund <andres@anarazel.de>; Dmitry Mityugov <d.mityugov@postgrespro.ru>; pgsql-hackers
Peter Geoghegan <pg@bowt.ie> writes:
> On Tue, Jul 15, 2025 at 7:27 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'd be a bit worried about
>> creating a back-patching mine-field. But maybe these are all
>> in spots we're unlikely to touch?
> That seems like much less of a problem for a purely subtractive change
> such as this.
Not really convinced. Taking a sample at random (from ExecRenameStmt
in alter.c):
return address;
}
default:
elog(ERROR, "unrecognized rename stmt type: %d",
(int) stmt->renameType);
return InvalidObjectAddress; /* keep compiler happy */
}
}
Assume we remove the "return InvalidObjectAddress;" line and later
need to back-patch a change touching this area.
If we were to add/change something in front of the "default:", we're
probably fine because the "default:" and the elog() would be enough
context lines to allow patch(1) to figure out where to put the
addition/change. However, if we wanted to add something after the
switch construct, we'd get an apply failure and have to fix it
manually --- or worse, patch(1) would apply the delta in the
wrong place.
I'm not sure how likely such scenarios are, but it doesn't seem
zero-risk. And if we do hundreds of these, the odds of trouble
will increase.
regards, tom lane
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-16 00:01 Peter Geoghegan <pg@bowt.ie>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 14+ messages in thread
From: Peter Geoghegan @ 2025-07-16 00:01 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; Dmitry Mityugov <d.mityugov@postgrespro.ru>; pgsql-hackers
On Tue, Jul 15, 2025 at 7:52 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I'm not sure how likely such scenarios are, but it doesn't seem
> zero-risk. And if we do hundreds of these, the odds of trouble
> will increase.
I agree that it's not zero-risk -- hardly anything ever is. Still
seems worth considering.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings
@ 2025-07-16 09:07 Peter Eisentraut <peter@eisentraut.org>
parent: Peter Geoghegan <pg@bowt.ie>
1 sibling, 0 replies; 14+ messages in thread
From: Peter Eisentraut @ 2025-07-16 09:07 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Dmitry Mityugov <d.mityugov@postgrespro.ru>; pgsql-hackers
On 16.07.25 01:35, Peter Geoghegan wrote:
> On Sat, Jul 12, 2025 at 1:55 PM Andres Freund <andres@anarazel.de> wrote:
>> I had played with using pg_assume here too, but I couldn't really convince
>> myself that it's a good idea...
>
> In the past, it was often necessary to work around MSVC's inability to
> see that a block containing elog(ERROR) doesn't actually need to
> initialize variables that'll never actually be used in code that comes
> after that block. We still have many "keep compiler quiet" variable
> initializations due to this.
>
> Is that still something that we need to worry about? I don't recall
> running into it in quite a few years, though that might just be a
> coincidence.
Yes, this continues to be an unsolved problem. See here for a recent
discussion:
https://www.postgresql.org/message-id/CAApHDvrFdXjbrV6KCx_GHKYSufUbNDYSsjppcJQiGOURfJE6qg@mail.gmail...
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2025-07-16 09:07 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-07-10 20:00 patch: Use pg_assume in jsonb_util.c to fix GCC 15 warnings Dmitry Mityugov <d.mityugov@postgrespro.ru>
2025-07-11 10:20 ` Erik Rijkers <er@xs4all.nl>
2025-07-12 17:42 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-07-12 17:55 ` Andres Freund <andres@anarazel.de>
2025-07-12 18:04 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-07-12 19:23 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-07-15 22:35 ` Peter Geoghegan <pg@bowt.ie>
2025-07-15 22:58 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-07-15 23:05 ` Peter Geoghegan <pg@bowt.ie>
2025-07-15 23:27 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-07-15 23:30 ` Peter Geoghegan <pg@bowt.ie>
2025-07-15 23:52 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-07-16 00:01 ` Peter Geoghegan <pg@bowt.ie>
2025-07-16 09:07 ` Peter Eisentraut <peter@eisentraut.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox