[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

Gábor Horváth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 3 14:47:02 PST 2019


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





More information about the cfe-commits mailing list