[PATCH] D53701: [Analyzer] Record and process comparison of symbols instead of iterator positions in interator checkers

Thu Dec 6 16:12:54 PST 2018

NoQ added a comment.

In D53701#1318566 <https://reviews.llvm.org/D53701#1318566>, @baloghadamsoftware wrote:

> In D53701#1315242 <https://reviews.llvm.org/D53701#1315242>, @NoQ wrote:
>
> >
>
>
> When using iterator adapters of the STL the bugs may also be filtered out by the analyzer if this option is enabled.

Mmm, are you talking about `c++-container-inlining`? This option currently prevents inlining of container methods. STL iterators shouldn't be affected, and even if they were affected, they will simply fall back to conservative evaluation.

On the other hand, this is how i *want* this option to work. I.e., instead of suppressing inlining, it should suppress bugs via visitors when an //interesting// event happen within a container, where "interesting" is potentially checker-specific and defined in every visitor separately. I believe that most checkers will be unaffected. Additionally, it still doesn't affect iterators.

> I think that only tracking the inner iterator and treating the outer iterator as a non-iterator is a nightmare from the user's perspective: all detections seem to be "internal" errors of the underlying API and thus regarded as "probably false positives". (...) The user must see the errors on the topmost level whenever possible to understand them.

Well, i mean, whenever you are inlining a method,  you //are// exposing details of the "internals" of the inlined API to the user. The only difference is whether this API itself deals with iterators. This sounds to me as if we try not to inline iterator methods in general. Or try really hard to prevent desynchronization between the two models.

Ok, how about an eager state split for now?

Repository:
  rC Clang

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

https://reviews.llvm.org/D53701