[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
Mon Aug 12 13:11:44 PDT 2019


Szelethus added a comment.

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

> In D65575#1625738 <https://reviews.llvm.org/D65575#1625738>, @Szelethus wrote:
>
> > A little early, gentle ping :) I'm getting kinda paranoid with the size of the stack, and how much I struggled with commiting last time.
>
>
> Mmm, what are your thoughts on my suggestions with wording? I'll also poke Devin more.


Pretty much this:

In D65575#1621425 <https://reviews.llvm.org/D65575#1621425>, @Szelethus wrote:

> 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.


I figured we'd have another round on this, but if it sounds good, I'll update the patch.

> Also it looks like most of the bottom of the stack is already accepted, you can just commit most of it, right?

Actually yea, I had a good reason to delay but not anymore.


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