While working on [1], I noticed some strange code in DiscreteKnapsack() which seems to be aiming to copy the Bitmapset.
It's not that obvious at a casual glance, but:
sets[j] = bms_del_members(sets[j], sets[j]);
this is aiming to zero all the words in the set by passing the same set in both parameters.
Now that 00b41463c changed Bitmapset to have NULL be the only valid representation of an empty set, this code no longer makes sense. We may as well just bms_free() the original set and bms_copy() in the new set as the bms_del_members() call will always pfree the set anyway.
I've done that in the attached.
+1. This is actually what happens with the original code, i.e. bms_del_members() will pfree sets[j] and bms_add_members() will bms_copy sets[ow] to sets[j]. But the new code looks more natural.
I also checked other callers of bms_del_members() and did not find another case that passes the same set in both parameters.
