[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D62883#1545341 <https://reviews.llvm.org/D62883#1545341>, @Szelethus wrote:

> In D62883#1545324 <https://reviews.llvm.org/D62883#1545324>, @Charusso wrote:
>
> > As @NoQ pointed out we have some problem with that function. We are tracking *values* without using the `redecl()`-chain (see in https://clang.llvm.org/doxygen/DeclBase_8h_source.html#l00948), or without tracking conditions. You have solved(?) the latter, but the former is equally important to solve.
>
>
> I'm a little confused, which is the "former" and "latter" you're referring to?


I believe you have solved the condition tracking as you move in-place what is going on.

>> I believe this patch should be on by default, but it requires to fix the "In which range do we track?" problem with D62978 <https://reviews.llvm.org/D62978>.
> 
> I disagree with this. The reason why I'd like to make this an off-by-default option is to implement my followup improvements incrementally (not only that, but a whole family of conditions is going to be added), and allow us to observe the changes those make in relation to this patch -- besides, I don't really see this patch changing even if I manage to fix those issues. Would you like to see a change being made to this specific patch?

As you moving stuff around it just bypasses the usefulness of the mentioned two visitors, which is a problem. Also you have mentioned some very crazy bad side-effects which is yes, related to in which range do we operate.


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

https://reviews.llvm.org/D62883