[PATCH] D31975: [Analyzer] Iterator Checkers

baloghadamsoftware added a comment. Split to 10 parts. https://reviews.llvm.org/D31975 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31975: [Analyzer] Iterator Checkers

NoQ added a comment. I've seen the new reviews. A lot of thanks for working on the patch split. I believe it'd eventually benefit everyone, similarly to how everybody benefits from code quality or something like that. I'd try to keep up with the "to split or not to split" discussion a bit below

[PATCH] D31975: [Analyzer] Iterator Checkers

baloghadamsoftware added a comment. Thank you for your help. Actually, I did neither merge the patches nor the checkers. Based on the experiences in the prototype I created a totally new checker. Incremental development sounds good, but I did a lot of experimenting in this particular checker,

[PATCH] D31975: [Analyzer] Iterator Checkers

NoQ added a comment. I had a look at how hard it is to split the patch. In ~3 hours i reached this far: F3239478: 0001-Delete-the-old-checker.patch F3239481: 0002-Reduced-patch-with-the-following-changes.patch F3239475: 000

[PATCH] D31975: [Analyzer] Iterator Checkers

baloghadamsoftware added a comment. Hello! I am not sure how to split it to incremental patches. As I mentioned in the description, the state of the iterator position is always tracked. So if I remove some of the checks it does not make the patch much smaller, since the checking part is quite s

[PATCH] D31975: [Analyzer] Iterator Checkers

NoQ added a comment. Hmm. 1. Could you do renaming the checker file in a separate patch, so that we saw an actual diff, not a whole greenish file, here on phabricator? 2. The invalidated iterator checker looks relatively small (a single check and a few rounds of iterator invalidations), would i

[PATCH] D31975: [Analyzer] Iterator Checkers

NoQ added a comment. I think for this case it'd be great to (instead) add comments all over the place (especially in checker callbacks, eg. `check[Pre|Post]Call()` function bodies) to indicate what check every piece of code is for. That'd be equally easy to review, at least for me, but it'd als

[PATCH] D31975: [Analyzer] Iterator Checkers

zaks.anna added a comment. @baloghadamsoftware Thanks for working on this! However, this patch is getting too big. Could you, please, split it into incremental patches so that it would be easier to review? More motivation of why this is very important is here http://llvm.org/docs/DeveloperPol