Re: Avoid undefined behavior with msvc compiler (src/include/port/pg_bitutils.h)

John Naylor <john(dot)naylor(at)enterprisedb(dot)com> writes:
> On Sun, Jul 30, 2023 at 9:45 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> That's basically equivalent to the existing Assert(non_zero).
>> I think it'd be okay to drop that one and instead have
>> the same Assert condition as other platforms, but having both
>> would be redundant.

> Works for me, so done that way for both forward and reverse variants. Since
> the return value is no longer checked in any builds, I thought about
> removing the variable containing it, but it seems best to leave it behind
> for clarity since these are not our functions.

Hmm, aren't you risking "variable is set but not used" warnings?
Personally I'd have made these like

(void) _BitScanReverse(&result, word);

Anybody trying to understand this code is going to have to look up
the man page for _BitScanReverse anyway, so I'm not sure that
keeping the variable adds much readability.

However, if no version of MSVC actually issues such a warning,
it's moot.

regards, tom lane