I'm actually curious about one particular mess that we have here. Namely, there's a visitor that says "assuming..." and there's checker notes when checkers themselves assume something; how can we be sure they don't duplicate each other?
BugReporter.cpp does some deduplication of notes originating from TrackConstraintBRVisitor and ConditionBRVisitor, but I'm not sure how many other visitors/checkers do we have doing the same.
|1898–1900 ↗||(On Diff #215294)|
When trackNulOrUndeflValue to trackExpressionValue, I think it was an oversight that the comments in it weren't changed accordingly. This makes a lot more sense now, cheers!
@Szelethus pointed out well. My patch is about nullability, and it is perfect. The bug report you made is about deduplication rather than nullability. The fact is we fail to call the deduplication function: eventsDescribeSameCondition() from removeRedundantMsgs(PathPieces &path) because the path does not contain the path pieces. We call removeRedundantMsgs with passing the PD->getMutablePieces(), but this only contains one piece of the foo function call in the test case you have attached.
I would upstream my hotfix of nullability without any tests as the comment says the intention and also we have plenty of tests of TrackConstraintBRVisitor, none of them changed.