Re: Simplifying our Trap/Assert infrastructure

2022-10-31 Thread Michael Paquier
On Mon, Oct 31, 2022 at 09:14:10AM -0400, Tom Lane wrote: > Peter Eisentraut writes: >> I don't think we need separate definitions for frontend and backend, >> since the contained Assert() will take care of the difference. So the >> attached would be simpler. > > WFM. Thanks, fine by me. -- M

Re: Simplifying our Trap/Assert infrastructure

2022-10-31 Thread Tom Lane
Peter Eisentraut writes: > I don't think we need separate definitions for frontend and backend, > since the contained Assert() will take care of the difference. So the > attached would be simpler. WFM. regards, tom lane

Re: Simplifying our Trap/Assert infrastructure

2022-10-31 Thread Peter Eisentraut
On 31.10.22 01:04, Michael Paquier wrote: On Fri, Oct 28, 2022 at 09:36:23AM +0200, Peter Eisentraut wrote: Would there be a use for that? It's currently only used in the atomics code. Yep, but they would not trigger when using atomics in the frontend code. We don't have any use for that in

Re: Simplifying our Trap/Assert infrastructure

2022-10-30 Thread Michael Paquier
On Fri, Oct 28, 2022 at 09:36:23AM +0200, Peter Eisentraut wrote: > Would there be a use for that? It's currently only used in the atomics > code. Yep, but they would not trigger when using atomics in the frontend code. We don't have any use for that in core on HEAD, still that could be useful f

Re: Simplifying our Trap/Assert infrastructure

2022-10-28 Thread Peter Eisentraut
On 27.10.22 09:23, Michael Paquier wrote: Agreed, even if extensions could use these, it looks like any out-of-core code using what's removed here would also gain in clarity. This is logically fine (except for an indentation blip in miscadmin.h?), so I have marked this entry as ready for committe

Re: Simplifying our Trap/Assert infrastructure

2022-10-27 Thread Michael Paquier
On Wed, Oct 12, 2022 at 09:19:17PM +0200, Peter Eisentraut wrote: > I'm in favor of this. These variants are a distraction. Agreed, even if extensions could use these, it looks like any out-of-core code using what's removed here would also gain in clarity. This is logically fine (except for an in

Re: Simplifying our Trap/Assert infrastructure

2022-10-12 Thread Peter Eisentraut
On 12.10.22 20:36, Nathan Bossart wrote: On Sun, Oct 09, 2022 at 02:01:48PM -0700, Nathan Bossart wrote: The patch LGTM. It might be worth removing usages of AssertArg and AssertState, too, but that can always be done separately. If you are so inclined... I'm in favor of this. These varian

Re: Simplifying our Trap/Assert infrastructure

2022-10-12 Thread Nathan Bossart
On Sun, Oct 09, 2022 at 02:01:48PM -0700, Nathan Bossart wrote: > The patch LGTM. It might be worth removing usages of AssertArg and > AssertState, too, but that can always be done separately. If you are so inclined... -- Nathan Bossart Amazon Web Services: https://aws.amazon.com >From 90e2166a

Re: Simplifying our Trap/Assert infrastructure

2022-10-10 Thread Tom Lane
Nathan Bossart writes: > On Sun, Oct 09, 2022 at 05:08:39PM -0400, Tom Lane wrote: >> Something I thought about but forgot to mention in the initial email: >> is it worth sprinkling these macros with "unlikely()"? > I don't see why not. I experimented with that, and found something that surprise

Re: Simplifying our Trap/Assert infrastructure

2022-10-09 Thread Nathan Bossart
On Sun, Oct 09, 2022 at 05:08:39PM -0400, Tom Lane wrote: > Something I thought about but forgot to mention in the initial email: > is it worth sprinkling these macros with "unlikely()"? I think that > compilers might assume the right thing automatically based on noticing > that ExceptionalConditi

Re: Simplifying our Trap/Assert infrastructure

2022-10-09 Thread Tom Lane
Nathan Bossart writes: > On Sun, Oct 09, 2022 at 03:51:57PM -0400, Tom Lane wrote: >> Hence, I propose the attached. > The patch LGTM. It might be worth removing usages of AssertArg and > AssertState, too, but that can always be done separately. Something I thought about but forgot to mention i

Re: Simplifying our Trap/Assert infrastructure

2022-10-09 Thread Nathan Bossart
On Sun, Oct 09, 2022 at 03:51:57PM -0400, Tom Lane wrote: > I happened to notice that the Trap and TrapMacro macros defined in c.h > have a grand total of one usage apiece across our entire code base. > It seems a little pointless and confusing to have them at all, since > they're essentially Asser

Simplifying our Trap/Assert infrastructure

2022-10-09 Thread Tom Lane
I happened to notice that the Trap and TrapMacro macros defined in c.h have a grand total of one usage apiece across our entire code base. It seems a little pointless and confusing to have them at all, since they're essentially Assert/AssertMacro but with the inverse condition polarity. I'm also a