Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 14dd0f2

Browse files
Add macros for looping through a List without a ListCell.
Many foreach loops only use the ListCell pointer to retrieve the content of the cell, like so: ListCell *lc; foreach(lc, mylist) { int myint = lfirst_int(lc); ... } This commit adds a few convenience macros that automatically declare the loop variable and retrieve the current cell's contents. This allows us to rewrite the previous loop like this: foreach_int(myint, mylist) { ... } This commit also adjusts a few existing loops in order to add coverage for the new/adjusted macros. There is presently no plan to bulk update all foreach loops, as that could introduce a significant amount of back-patching pain. Instead, these macros are primarily intended for use in new code. Author: Jelte Fennema-Nio Reviewed-by: David Rowley, Alvaro Herrera, Vignesh C, Tom Lane Discussion: https://postgr.es/m/CAGECzQSwXKnxGwW1_Q5JE%2B8Ja20kyAbhBHO04vVrQsLcDciwXA%40mail.gmail.com
1 parent 5e8674d commit 14dd0f2

File tree

5 files changed

+71
-26
lines changed

5 files changed

+71
-26
lines changed

src/backend/executor/execExpr.c

+3-6
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@ ExecInitQual(List *qual, PlanState *parent)
216216
ExprState *state;
217217
ExprEvalStep scratch = {0};
218218
List *adjust_jumps = NIL;
219-
ListCell *lc;
220219

221220
/* short-circuit (here and in ExecQual) for empty restriction list */
222221
if (qual == NIL)
@@ -250,10 +249,8 @@ ExecInitQual(List *qual, PlanState *parent)
250249
scratch.resvalue = &state->resvalue;
251250
scratch.resnull = &state->resnull;
252251

253-
foreach(lc, qual)
252+
foreach_ptr(Expr, node, qual)
254253
{
255-
Expr *node = (Expr *) lfirst(lc);
256-
257254
/* first evaluate expression */
258255
ExecInitExprRec(node, state, &state->resvalue, &state->resnull);
259256

@@ -265,9 +262,9 @@ ExecInitQual(List *qual, PlanState *parent)
265262
}
266263

267264
/* adjust jump targets */
268-
foreach(lc, adjust_jumps)
265+
foreach_int(jump, adjust_jumps)
269266
{
270-
ExprEvalStep *as = &state->steps[lfirst_int(lc)];
267+
ExprEvalStep *as = &state->steps[jump];
271268

272269
Assert(as->opcode == EEOP_QUAL);
273270
Assert(as->d.qualexpr.jumpdone == -1);

src/backend/replication/logical/relation.c

+1-3
Original file line numberDiff line numberDiff line change
@@ -746,11 +746,9 @@ static Oid
746746
FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
747747
{
748748
List *idxlist = RelationGetIndexList(localrel);
749-
ListCell *lc;
750749

751-
foreach(lc, idxlist)
750+
foreach_oid(idxoid, idxlist)
752751
{
753-
Oid idxoid = lfirst_oid(lc);
754752
bool isUsableIdx;
755753
Relation idxRel;
756754
IndexInfo *idxInfo;

src/backend/replication/logical/tablesync.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -1036,11 +1036,11 @@ fetch_remote_table_info(char *nspname, char *relname,
10361036

10371037
/* Build the pubname list. */
10381038
initStringInfo(&pub_names);
1039-
foreach(lc, MySubscription->publications)
1039+
foreach_node(String, pubstr, MySubscription->publications)
10401040
{
1041-
char *pubname = strVal(lfirst(lc));
1041+
char *pubname = strVal(pubstr);
10421042

1043-
if (foreach_current_index(lc) > 0)
1043+
if (foreach_current_index(pubstr) > 0)
10441044
appendStringInfoString(&pub_names, ", ");
10451045

10461046
appendStringInfoString(&pub_names, quote_literal_cstr(pubname));

src/backend/replication/pgoutput/pgoutput.c

+3-4
Original file line numberDiff line numberDiff line change
@@ -2234,7 +2234,6 @@ cleanup_rel_sync_cache(TransactionId xid, bool is_commit)
22342234
{
22352235
HASH_SEQ_STATUS hash_seq;
22362236
RelationSyncEntry *entry;
2237-
ListCell *lc;
22382237

22392238
Assert(RelationSyncCache != NULL);
22402239

@@ -2247,15 +2246,15 @@ cleanup_rel_sync_cache(TransactionId xid, bool is_commit)
22472246
* corresponding schema and we don't need to send it unless there is
22482247
* any invalidation for that relation.
22492248
*/
2250-
foreach(lc, entry->streamed_txns)
2249+
foreach_xid(streamed_txn, entry->streamed_txns)
22512250
{
2252-
if (xid == lfirst_xid(lc))
2251+
if (xid == streamed_txn)
22532252
{
22542253
if (is_commit)
22552254
entry->schema_sent = true;
22562255

22572256
entry->streamed_txns =
2258-
foreach_delete_current(entry->streamed_txns, lc);
2257+
foreach_delete_current(entry->streamed_txns, streamed_txn);
22592258
break;
22602259
}
22612260
}

src/include/nodes/pg_list.h

+61-10
Original file line numberDiff line numberDiff line change
@@ -381,26 +381,26 @@ lnext(const List *l, const ListCell *c)
381381
/*
382382
* foreach_delete_current -
383383
* delete the current list element from the List associated with a
384-
* surrounding foreach() loop, returning the new List pointer.
384+
* surrounding foreach() or foreach_*() loop, returning the new List
385+
* pointer; pass the name of the iterator variable.
385386
*
386-
* This is equivalent to list_delete_cell(), but it also adjusts the foreach
387-
* loop's state so that no list elements will be missed. Do not delete
388-
* elements from an active foreach loop's list in any other way!
387+
* This is similar to list_delete_cell(), but it also adjusts the loop's state
388+
* so that no list elements will be missed. Do not delete elements from an
389+
* active foreach or foreach_* loop's list in any other way!
389390
*/
390-
#define foreach_delete_current(lst, cell) \
391-
(cell##__state.i--, \
392-
(List *) (cell##__state.l = list_delete_cell(lst, cell)))
391+
#define foreach_delete_current(lst, var_or_cell) \
392+
((List *) (var_or_cell##__state.l = list_delete_nth_cell(lst, var_or_cell##__state.i--)))
393393

394394
/*
395395
* foreach_current_index -
396-
* get the zero-based list index of a surrounding foreach() loop's
397-
* current element; pass the name of the "ListCell *" iterator variable.
396+
* get the zero-based list index of a surrounding foreach() or foreach_*()
397+
* loop's current element; pass the name of the iterator variable.
398398
*
399399
* Beware of using this after foreach_delete_current(); the value will be
400400
* out of sync for the rest of the current loop iteration. Anyway, since
401401
* you just deleted the current element, the value is pretty meaningless.
402402
*/
403-
#define foreach_current_index(cell) (cell##__state.i)
403+
#define foreach_current_index(var_or_cell) (var_or_cell##__state.i)
404404

405405
/*
406406
* for_each_from -
@@ -452,6 +452,57 @@ for_each_cell_setup(const List *lst, const ListCell *initcell)
452452
return r;
453453
}
454454

455+
/*
456+
* Convenience macros that loop through a list without needing a separate
457+
* "ListCell *" variable. Instead, the macros declare a locally-scoped loop
458+
* variable with the provided name and the appropriate type.
459+
*
460+
* Since the variable is scoped to the loop, it's not possible to detect an
461+
* early break by checking its value after the loop completes, as is common
462+
* practice. If you need to do this, you can either use foreach() instead or
463+
* manually track early breaks with a separate variable declared outside of the
464+
* loop.
465+
*
466+
* Note that the caveats described in the comment above the foreach() macro
467+
* also apply to these convenience macros.
468+
*/
469+
#define foreach_ptr(type, var, lst) foreach_internal(type, *, var, lst, lfirst)
470+
#define foreach_int(var, lst) foreach_internal(int, , var, lst, lfirst_int)
471+
#define foreach_oid(var, lst) foreach_internal(Oid, , var, lst, lfirst_oid)
472+
#define foreach_xid(var, lst) foreach_internal(TransactionId, , var, lst, lfirst_xid)
473+
474+
/*
475+
* The internal implementation of the above macros. Do not use directly.
476+
*
477+
* This macro actually generates two loops in order to declare two variables of
478+
* different types. The outer loop only iterates once, so we expect optimizing
479+
* compilers will unroll it, thereby optimizing it away.
480+
*/
481+
#define foreach_internal(type, pointer, var, lst, func) \
482+
for (type pointer var = 0, pointer var##__outerloop = (type pointer) 1; \
483+
var##__outerloop; \
484+
var##__outerloop = 0) \
485+
for (ForEachState var##__state = {(lst), 0}; \
486+
(var##__state.l != NIL && \
487+
var##__state.i < var##__state.l->length && \
488+
(var = func(&var##__state.l->elements[var##__state.i]), true)); \
489+
var##__state.i++)
490+
491+
/*
492+
* foreach_node -
493+
* The same as foreach_ptr, but asserts that the element is of the specified
494+
* node type.
495+
*/
496+
#define foreach_node(type, var, lst) \
497+
for (type * var = 0, *var##__outerloop = (type *) 1; \
498+
var##__outerloop; \
499+
var##__outerloop = 0) \
500+
for (ForEachState var##__state = {(lst), 0}; \
501+
(var##__state.l != NIL && \
502+
var##__state.i < var##__state.l->length && \
503+
(var = lfirst_node(type, &var##__state.l->elements[var##__state.i]), true)); \
504+
var##__state.i++)
505+
455506
/*
456507
* forboth -
457508
* a convenience macro for advancing through two linked lists

0 commit comments

Comments
 (0)