xazax.hun added a comment. In D70725#1767942 <https://reviews.llvm.org/D70725#1767942>, @NoQ wrote:
> Mmm, right, i guess you should also skip adding the tag if the notes array is > empty. > > There might be more complicated use-cases (especially in `ExprEngine` itself) > where you may end up adding a tag even if the state doesn't change. For > instance, when changing an Environment value from absent to `UnknownVal`, the > state doesn't get actually updated. Or making range assumptions over > `UnknownVal`. But i'd argue that we do indeed want the note tag in such cases > because it is still worth annotating the `ExplodedGraph` with an explanation > of what's happening. Eg., a note "Assuming a condition is false" makes sense > even if the condition is an `UnknownVal` and no actual constraints are added. > > Another example: in a recent `StreamChecker` review we've been talking about > a peculiar stream function that by design closes a file descriptor if it's > open but does nothing if the descriptor is already closed. I believe this > event deserves a note "The descriptor remains closed" (possibly prunable) > even though there is no change in the state. This is something we couldn't do > with our usual visitor-based approach which involves observing changes in the > state (we technically could, via pattern-matching over the program point, but > that'd directly duplicate a lot more of checker logic than usual). I see, that makes sense. I updated the code. I was hoping for a low hanging fruit to make this more user friendly :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
