[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-02-06 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D29151#668197, @zaks.anna wrote: > > I tried to come up with some advice on where checks should go in > > http://clang.llvm.org/extra/clang-tidy/#choosing-the-right-place-for-your-check > > The guidelines stated on the clang-tidy page seem reas

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-02-06 Thread Anna Zaks via Phabricator via cfe-commits
zaks.anna added a comment. > I tried to come up with some advice on where checks should go in > http://clang.llvm.org/extra/clang-tidy/#choosing-the-right-place-for-your-check The guidelines stated on the clang-tidy page seem reasonable to me. > For example, IMO, AST-based analyses make more se

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-02-04 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D29151#665948, @alexfh wrote: > In https://reviews.llvm.org/D29151#662504, @zaks.anna wrote: > > > Before clang-tidy came into existence the guidelines were very clear. One > > should write a clang warning if the diagnostic: > > > > - can be im

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-02-03 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D29151#662504, @zaks.anna wrote: > Before clang-tidy came into existence the guidelines were very clear. One > should write a clang warning if the diagnostic: > > - can be implemented without path-insensitive analysis (including > flow-sensiti

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-01-31 Thread Anna Zaks via Phabricator via cfe-commits
zaks.anna added a comment. Before clang-tidy came into existence the guidelines were very clear. One should write a clang warning if the diagnostic: - can be implemented without path-insensitive analysis (including flow-sensitive) - is checking for language rules (not specific API misuse) In m

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-01-31 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. > I think we need some sort of clear guidelines on where what functionality > should be added. Even right now there are clang-tidy checks that finds subset > of alpha checks, but probably having lower false positive rate. The other > example is Use after move, th

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-01-31 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D29151#658484, @zaks.anna wrote: > The static analyzer is definitely the place to go for bug detection that > requires path sensitivity. It's also reasonably good for anything that needs > flow-sensitivity. Although, most flow-sensitive analys

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-01-26 Thread Anna Zaks via Phabricator via cfe-commits
zaks.anna added a comment. The static analyzer is definitely the place to go for bug detection that requires path sensitivity. It's also reasonably good for anything that needs flow-sensitivity. Although, most flow-sensitive analysis (such as liveness) now live in lib/Analysis/, which is used b

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-01-26 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. In https://reviews.llvm.org/D29151#657435, @Prazek wrote: > In https://reviews.llvm.org/D29151#656887, @Eugene.Zelenko wrote: > > > General question: isn't Static Analyzer is better place for such workflow > > checks? > > > Probably yes, but it is much harder to i

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-01-26 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added a comment. In https://reviews.llvm.org/D29151#656887, @Eugene.Zelenko wrote: > General question: isn't Static Analyzer is better place for such workflow > checks? Probably yes, but it is much harder to implement this there. Other problem is that it would be probably a part of on

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-01-25 Thread Piotr Padlewski via Phabricator via cfe-commits
Prazek added inline comments. Comment at: clang-tidy/misc/InvalidatedIteratorsCheck.cpp:61 + cxxMemberCallExpr(has(memberExpr(hasDeclaration(cxxMethodDecl(hasAnyName( + "push_back", "emplace_back", "clear", +

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

2017-01-25 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. General question: isn't Static Analyzer is better place for such workflow checks? Repository: rL LLVM https://reviews.llvm.org/D29151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c