[PATCH] D73547: [Analyzer] Split container modeling from iterator modeling

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

I'm late to the party, but have looked at the code and I really, really-really 
like what you did in this patch! It solves one of my biggest complaints about 
this project. LGTM!




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:1389
   HelpText<"Check the analyzer's understanding of C++ iterators">,
-  Dependencies<[IteratorModeling]>,
+  Dependencies<[DebugContainerModeling, IteratorModeling]>,
   Documentation;

This just works??? I admit that I didn't test very thoroughly whether multiple 
dependencies would work out, but I'm glad it does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73547



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73547: [Analyzer] Split container modeling from iterator modeling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Ouch, whoops, i didn't notice the new test file. LGTM then!


Repository:
  rC Clang

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

https://reviews.llvm.org/D73547



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73547: [Analyzer] Split container modeling from iterator modeling

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In D73547#1845339 , @NoQ wrote:

> This patch is simply moving code around, right? If so, why did tests need to 
> be removed?


All of the container begin- and end-tests were removed from 
`iterator-modeling.cpp` and moved to `container-modelin.cpp` except those which 
do not make sense at the moment: container ends are not touched in modeling 
`insert()` and `erase()` methods. This will probably change in the future but I 
cannot even add `FIXME` tests because I do not know every detail yet. If you 
which, I can add these tests back to `container-modeling.cpp` as they were in 
`iterator-modeling.cpp`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D73547



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73547: [Analyzer] Split container modeling from iterator modeling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

This patch is simply moving code around, right? If so, why did tests need to be 
removed?


Repository:
  rC Clang

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

https://reviews.llvm.org/D73547



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits