Committed after some additional light edits. Thanks for the patch!
On Wed, Jan 03, 2024 at 11:36:36PM +0100, Jelte Fennema-Nio wrote:
> On Wed, 3 Jan 2024 at 23:13, Tom Lane wrote:
>> I like Nathan's wording.
>
> To be clear, I don't want to block this patch on the wording of that
> single comm
On Wed, 3 Jan 2024 at 23:13, Tom Lane wrote:
> I like Nathan's wording.
To be clear, I don't want to block this patch on the wording of that
single comment. So, if you feel Nathan's wording was better, I'm fine
with that too. But let me respond to your arguments anyway:
> Your assertion is contr
Jelte Fennema-Nio writes:
> The "we expect" reads to me as if we're not very sure that compilers
> do this optimization. Even though we are quite sure. Maybe some small
> changes like this to clarify that.
> The outer loop only does a single iteration, so we expect that **any**
> optimizing compi
On Wed, Jan 03, 2024 at 10:57:07PM +0100, Jelte Fennema-Nio wrote:
> Overall your light edits look good to me. The commit message is very
> descriptive and I like the shortening of the comments. The only thing
> I feel is that I think lost some my original intent is this sentence:
>
> + * differen
On Wed, 3 Jan 2024 at 20:55, Nathan Bossart wrote:
>
> I spent some time preparing this for commit, which only amounted to some
> light edits. I am posting a new version of the patch in order to get one
> more round of cfbot coverage and to make sure there is no remaining
> feedback.
Overall you
I spent some time preparing this for commit, which only amounted to some
light edits. I am posting a new version of the patch in order to get one
more round of cfbot coverage and to make sure there is no remaining
feedback.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From f91c
On Wed, Dec 20, 2023 at 12:21:05PM +0530, vignesh C wrote:
> On Tue, 19 Dec 2023 at 21:22, Nathan Bossart wrote:
>> I'm not sure we should proceed with rewriting most/all eligible foreach
>> loops. I think it's fine to use the new macros in new code or to update
>> existing loops in passing when
On Tue, 19 Dec 2023 at 21:22, Nathan Bossart wrote:
>
> On Tue, Dec 19, 2023 at 03:44:43PM +0100, Jelte Fennema-Nio wrote:
> > On Tue, 19 Dec 2023 at 11:59, vignesh C wrote:
> >> I noticed that this change can be done in several other places too.
> >
> > My guess would be that ~90% of all existin
On Tue, 19 Dec 2023 at 16:52, Nathan Bossart wrote:
> I'm not sure we should proceed with rewriting most/all eligible foreach
> loops. I think it's fine to use the new macros in new code or to update
> existing loops in passing when changing nearby code, but rewriting
> everything likely just int
On Tue, Dec 19, 2023 at 03:44:43PM +0100, Jelte Fennema-Nio wrote:
> On Tue, 19 Dec 2023 at 11:59, vignesh C wrote:
>> I noticed that this change can be done in several other places too.
>
> My guess would be that ~90% of all existing foreach loops in the
> codebase can be easily rewritten (and s
On Tue, 19 Dec 2023 at 11:59, vignesh C wrote:
> I noticed that this change can be done in several other places too.
My guess would be that ~90% of all existing foreach loops in the
codebase can be easily rewritten (and simplified) using these new
macros. So converting all of those would likely b
On Mon, 18 Dec 2023 at 19:00, Jelte Fennema-Nio wrote:
>
> The more I think about it and look at the code, the more I like the
> usage of the loop style proposed in the previous 0003 patch (which
> automatically declares a loop variable for the scope of the loop using
> a second for loop).
>
> I d
The more I think about it and look at the code, the more I like the
usage of the loop style proposed in the previous 0003 patch (which
automatically declares a loop variable for the scope of the loop using
a second for loop).
I did some testing on godbolt.org and both versions of the macros
result
On Fri, 1 Dec 2023 at 05:20, Nathan Bossart wrote:
> Could we simplify it with something like the following?
Great suggestion! Updated the patchset accordingly.
This made it also easy to change the final patch to include the
automatic scoped declaration logic for all of the new macros. I quite
l
On Wed, Oct 25, 2023 at 12:39:01PM +0200, Jelte Fennema wrote:
> Attached is a slightly updated version, with a bit simpler
> implementation of foreach_delete_current.
> Instead of decrementing i and then adding 1 to it when indexing the
> list, it now indexes the list using a postfix decrement.
B
On Wed, 25 Oct 2023 at 13:52, Alvaro Herrera wrote:
> Looking at for_each_ptr() I think it may be cleaner to follow
> palloc_object()'s precedent and make it foreach_object() instead (I have
> no love for the extra underscore, but I won't object to it either). And
> like foreach_node, have it rec
On 2023-Oct-24, Jelte Fennema wrote:
> Many usages of the foreach macro in the Postgres codebase only use the
> ListCell variable to then get its value. This adds macros that
> simplify iteration code for that very common use case. Instead of
> passing a ListCell you can pass a variable of the typ
Attached is a slightly updated version, with a bit simpler
implementation of foreach_delete_current.
Instead of decrementing i and then adding 1 to it when indexing the
list, it now indexes the list using a postfix decrement.
From 9073777a6d82e2b2db8b1ed9aef200550234d89a Mon Sep 17 00:00:00 2001
Fr
On Wed, 25 Oct 2023 at 04:55, David Rowley wrote:
> With foreach(), we commonly do "if (lc == NULL)" at the end of loops
> as a way of checking if we did "break" to terminate the loop early.
Afaict it's done pretty infrequently. The following crude attempt at
an estimate estimates it's only done
On Wed, 25 Oct 2023 at 06:00, Jelte Fennema wrote:
> Attached is a fixed version.
With foreach(), we commonly do "if (lc == NULL)" at the end of loops
as a way of checking if we did "break" to terminate the loop early.
Doing the equivalent with the new macros won't be safe as the list
element's v
On Tue, Oct 24, 2023 at 06:58:04PM +0200, Jelte Fennema wrote:
> On Tue, 24 Oct 2023 at 18:47, Nathan Bossart wrote:
>> BTW after applying your patches, initdb began failing with the following
>> for me:
>>
>> TRAP: failed Assert("n >= 0 && n < list->length"), File: "list.c",
>> Line: 770
On Tue, 24 Oct 2023 at 18:47, Nathan Bossart wrote:
> BTW after applying your patches, initdb began failing with the following
> for me:
>
> TRAP: failed Assert("n >= 0 && n < list->length"), File: "list.c",
> Line: 770, PID: 902807
Oh oops... That was an off by one error in the modified
On Tue, Oct 24, 2023 at 06:03:48PM +0200, Jelte Fennema wrote:
> Many usages of the foreach macro in the Postgres codebase only use the
> ListCell variable to then get its value. This adds macros that
> simplify iteration code for that very common use case. Instead of
> passing a ListCell you can p
Many usages of the foreach macro in the Postgres codebase only use the
ListCell variable to then get its value. This adds macros that
simplify iteration code for that very common use case. Instead of
passing a ListCell you can pass a variable of the type of its
contents. This IMHO improves readabil
24 matches
Mail list logo
