If we have an unknown value do not treat it as known.
Ah, that positive!
No, i don't think this is a valid way to suppress it. I'll tease you a bit more though, and only give a hint: the null value that we're null-dereferencing was in fact assumed to be null in the top frame, not within any inline function. Therefore the whole "Returning pointer..." note is completely misleading: it is irrelevant what value we return from getSuperRegion(), we might have as well evaluated it conservatively, the report would still have been emitted.
Positive == true positive, not false positive, I got it.
No, i don't think this is a valid way to suppress it.
Bought me, they are worth to report. The misleading reports made me think I have to suppress them.
Example (difference starts at note 33, at line 410):
This remaining note is also unnecessary. You can safely stop tracking the value at e || .... In particular, ReturnVisitor is not at fault.
That said, when we renamed trackNullOrUndefValue to trackExpressionValue, we kinda assumed that it's not really important whether the value is null/undef or not - the function simply tracks the value. This change would break this invariant, causing null values to be handled in a special manner. I recommend adding another flag argument (similar to EnableNullFPSuppression) that would allow the caller to tell whether it's interested in the null or in the "whole" value (defaulting to the latter).
That is a great idea! I tried my best but after a while I give up because it is already too complex, so I just removed them. It turns out it makes sense to remove those notes, see inline.
Here we mismatch the parameter.
This problem is fairly complicated. We clearly need both behaviors (sometimes track until the definition, sometimes track until the collapse-to-null), and it's not clear to me right now when exactly do we need each of them. This is also not a high priority for GSoC because neither there are a lot of warnings of this kind (~15 or so) nor they're actually that false. I suggest taking this more slowly and delay this patch until we actually understand what is the right thing to do here.
This note was pretty good. It's not very clear but we should definitely keep it.
These notes are also nice, we should keep them.