Re: cleanup in code

On 2014-01-06 23:51:52 +1300, David Rowley wrote:
> On Mon, Jan 6, 2014 at 11:38 PM, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com
> > wrote:
>
> > On 01/04/2014 07:20 AM, Amit Kapila wrote:
> >
> >> 1. compiling with msvc shows warning in relcache.c
> >> 1>e:\workspace\postgresql\master\postgresql\src\backend\
> >> utils\cache\relcache.c(3959):
> >> warning C4715: 'RelationGetIndexAttrBitmap' : not all control paths
> >> return a value
> >>
> >> Attached patch remove_msvc_warning.patch to remove above warning
> >>
> >
> > Hmm, I thought we gave enough hints in the elog macro to tell the compiler
> > that elog(ERROR) does no return, since commit b853eb97182079dcd30b4f52576bd5d6c275ee71.
> > Have we not enabled that for MSVC?
> >
> >
> I looked at this a while back here:
> http://www.postgresql.org/message-id/CAApHDvqOsb4nc3OG0xoBoJ2fmA-6AkihuWsAd43RLekqk6SmCQ@mail.gmail.com
>
> And found that because elevel was being assigned to a variable that the
> compiler could not determine that the if (elevel_ >= ERROR) was constant
> therefore couldn't assume that __assume(0) would be reached with the
> microsoft compiler

But afair the declaration for elog() works in several other places, so
that doesn't sufficiently explain this. I'd very much expect that that
variable is complitely elided by any halfway competent compiler - it's
just there to prevent multiple evaluation should elevel not be a
constant.
Do you see the warning both with asserts enabled and non-assert builds?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services