Re: Warning in the RecordTransactionAbort routine during compilation with O3 flag

On Mon, Dec 09, 2019 at 02:03:43PM +0500, Andrey Lepikhov wrote:
> We already have assertion on the twophase_gid variable. But compiler is not
> so smart and can't find a link between the XACT_XINFO_HAS_GID flag and
> state of twophase_gid pointer.

Well, gcc-9 got visibly smarter on that point :)

> Ok. In accordance with your review, I have prepared a new version of the
> patch.

Regarding formatting.c, I can see the point of avoiding future
mistakes, and I would go a bit further as per the attached for
consistency between all variables.  have_error is a bit trickier
though as it gets moved around more layers so doing an initialization
in the middle is not really an option.  Anyway, we can do that rather
cleanly from the entry point of do_to_timestamp() to bring more
consistency for variables which are always expected and the optional
ones.  What do you think?

For the second one in xact.c, I am not really on board of doing
something based on the proposals because this reduces the code
visibility, and gcc is clearly wrong in its assumptions because the
state cannot be reached.

> My compiler:
> 
> gcc -v
> [...]
> gcc version 7.4.0 (Ubuntu 7.4.0-1ubuntu1~18.04.1)

Thanks, that's the difference.  gcc-9 does not complain, but I can see
the warnings with gcc-7 (7.5.0 actually).
--
xMichael