Although nsInterfaceHashtable.Init returns bool, nsBoxObject::SetPropertyAsSupports uses NS_FAILED. NS_FAILED(bool) is always true.
So this used to always test false, and now always tests true?
That's really bad...
Michael, could we write some sort of static analysis to catch other cases like this?
(In reply to Boris Zbarsky (:bz) from comment #1)
> So this used to always test false, and now always tests true?
As far as I can tell, this always tested false, and will continue to test false. The underlying implementation (nsTHashtable) returns true for success and false for failure, so the switch to bool cannot change anything as far as I can tell.
> That's really bad...
> Michael, could we write some sort of static analysis to catch other cases
> like this?
MSVC actually tells us if we're trying to test bits in a bool which cannot be anything but a zero.
> As far as I can tell, this always tested false, and will continue to test false
Ok, then there's no real issue. Comment 0 was implying that NS_FAILED(bool) would be true for some reason, so I assumed there was something about the C++ treatment of bool that I was missing....
I did just verify that bool(true & 0x80000000) is false, and likewise for false. Sorry for the false alarm. :(
Created attachment 592595 [details] [diff] [review]
Comment on attachment 592595 [details] [diff] [review]