[PATCH] D65575: [analyzer] Mention whether an event is about a condition in a bug report part 1

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 8 10:29:44 PDT 2019


Szelethus added a comment.

In D65575#1611013 <https://reviews.llvm.org/D65575#1611013>, @NoQ wrote:

> Fantastic! Let's open the wording bikeshed season?
>
> I suspect that a simple "(The) Value -> Condition value" change would have worked better.
>
> Another variant: "Value ..., which participates in a condition later".


Yea, I kinda prefer a more uniform indication as to whether we're explaining a condition or "THE value". While I personally took a unique approach in evaluating analysis results (my eye was hunting for the changes I made specifically), I did find each function call in the bug report super easy to understand:
F9758175: image.png <https://reviews.llvm.org/F9758175>
See how this function call screams what it is about? Now, condition tracking is inherently imperfect (like bug report construction as a whole), and whenever I feel like the notes added by it provide little value, simply glancing at the notes can tell whether I should observe that function call or not.

I think it isn't crucial of getting rid of the "The" prefix, if we append ", which participates in a condition later" (which sounds so much better than what I added in this patch), so maybe changing `WillBeUsedForACondition` to that would be good enough. However, as I said, I realize that the way I looked at these results was a lot different than how the average user will do so, so I'm totally open on this topic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65575/new/

https://reviews.llvm.org/D65575





More information about the cfe-commits mailing list