useless assignment pointer argument

Started by Gaetano Mendolaover 10 years ago4 messages
#1Gaetano Mendola
mendola@gmail.com

Hi,
in the following spots:

src/backend/commands/explain.c:1692
src/backend/commands/explain.c:1874
src/backend/commands/explain.c:1986

there is the following assignment:

ancestors = list_delete_first(ancestors);

but it has no effect at all being that a function parameter and not used
anymore after the assignment itself.

#2Andres Freund
andres@anarazel.de
In reply to: Gaetano Mendola (#1)
Re: useless assignment pointer argument

On 2015-05-28 20:14:33 +0000, Gaetano Mendola wrote:

src/backend/commands/explain.c:1692
src/backend/commands/explain.c:1874
src/backend/commands/explain.c:1986

there is the following assignment:

ancestors = list_delete_first(ancestors);

but it has no effect at all being that a function parameter and not used
anymore after the assignment itself.

So? It costs little to nothing, and it'll make it much less likely that
a stale version of 'ancestors' is used when the code is expanded.

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Gaetano Mendola
mendola@gmail.com
In reply to: Andres Freund (#2)
Re: useless assignment pointer argument

If the compiler is good the assignment is elided indeed, that's not what I
meant to point out.

On Thu, 28 May 2015 at 22:17 Andres Freund <andres@anarazel.de> wrote:

Show quoted text

On 2015-05-28 20:14:33 +0000, Gaetano Mendola wrote:

src/backend/commands/explain.c:1692
src/backend/commands/explain.c:1874
src/backend/commands/explain.c:1986

there is the following assignment:

ancestors = list_delete_first(ancestors);

but it has no effect at all being that a function parameter and not used
anymore after the assignment itself.

So? It costs little to nothing, and it'll make it much less likely that
a stale version of 'ancestors' is used when the code is expanded.

Greetings,

Andres Freund

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#2)
Re: useless assignment pointer argument

Andres Freund <andres@anarazel.de> writes:

On 2015-05-28 20:14:33 +0000, Gaetano Mendola wrote:

src/backend/commands/explain.c:1692
src/backend/commands/explain.c:1874
src/backend/commands/explain.c:1986

there is the following assignment:

ancestors = list_delete_first(ancestors);

but it has no effect at all being that a function parameter and not used
anymore after the assignment itself.

So? It costs little to nothing, and it'll make it much less likely that
a stale version of 'ancestors' is used when the code is expanded.

It's completely mistaken to imagine that the statement has no effect:
it does change the underlying List structure, so removing it would be
wrong.

It's true that we could, today, write it as

(void) list_delete_first(ancestors);

but this is not any more clear IMO, and it would fail if the code were
ever changed so that these functions did use the ancestors list after
the point of popping the context they pushed for their children. That
risk outweighs any argument about how deleting the assignment would
quiet some tool or other.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers