warn_impcast_null_pointer_to_integer (" implicit conversion of NULL constant to integer"), a warning that is meant to trigger whenever NULL is used in a non-pointer context, was not firing whenever the integer type it was converted to was the same size as the pointer on the given platform. <div>
<br></div><div>A __null expression was of type 'int' on 32 bit platforms and 'long' on 64 bit platforms, etc. This meant that there was no conversion between __null and int/long, as they were the exact same type. So the check for warn_impcast_null_pointer_to_integer never fired, since it happened afterwards.</div>
<div><br></div><div>To fix this I moved the check up to the top of the main CheckImplicitConversions function & had to remove a shortcut check in one of the callers (this was the check for E.getType() == T that was causing the warning to be missed, though just removing that wouldn't've been sufficient since there was another similar check at the start of CheckImplicitConversions).<br>
<br>I've updated the test case only slightly to add one case that would've failed without this change "long x = NULL;" (the conversions.cpp only compiles as 64 bit, and since all the tests for NULL conversion were using int they never hit this bug. I'm not sure if it's worth testing all the cases already covered but in a 32 bit build too - maybe at least a quick sanity check might be nice (my own personal test case was to use short, int, and long initialized with NULL, assigned with NULL, then a function that accepted all 3 types that was passed NULL to all of them))<br>
<br>This solution might be a bit ham-fisted as I'm not sure which operations are the cheapest - between type comparisons, type canonicalization, etc. There's a semi-redundant check for E->getType() == T - a check, then a type canonicalization, then a check on the canonicalized type. If canonicalizing the types is cheap enough, the check before canonicalization could be removed.</div>
<div><br></div><div>I also simplified the GNUNull check, not just for performance but correctness. In one particular case (reinterpret_cast<int&&>('a')) trying to call isNullPointerConstant on that whole cast Expression crashes because it cannot be evaluated (perhaps this should be corrected, but it wasn't necessary for now).</div>
<div><br></div><div>[after this I'll add a fixit to replace NULL with the correct integer type zero value]</div><div><br></div><div>Thanks for your time,</div><div>- David</div>