John Naylor <john.nay...@enterprisedb.com> writes:
> On Sun, Jul 30, 2023 at 9:45 PM Tom Lane <t...@sss.pgh.pa.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
