Re: cleanup in code

On Mon, Jan 6, 2014 at 4:08 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 think it is enabled as part of commit
52906f175a05a4e7e5e4a0e6021c32b1bfb221cf.

The reason why we are seeing this warning is it doesn't reach pg_unreachable due
to if loop. I think the reason mention by David is right.

However if I just change the code of elog to not use elevel_ variable
it works fine.
Changing code like below removes warning and passes all regression on windows.
#define elog(elevel, ...) \
do { \
elog_start(__FILE__, __LINE__, PG_FUNCNAME_MACRO); \
elog_finish(elevel, __VA_ARGS__); \
if (elevel >= ERROR) \
pg_unreachable(); \
} while(0)

Having said above, I think there must be a reason to have elevel_ which I am
not aware.

> Do you see the warning both with asserts enabled and non-assert builds?

Yes.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com