[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
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 reasonable to me. > > > For example, IMO, AST-based analyses make more sense in clang-tidy for a > > few reasons: > > > > They usually are easier expressible in terms of AST matchers, and > > clang-tidy allows to use AST matchers with less boilerplate. > > Clang Static Analyzer is linked into clang, where AST matchers are > > undesired, since they tend to blow the size of binary significantly. > > It's more consistent to keep all similar analyses together, it simplifies > > search for already implemented similar functionality and code reviews. > > I agree that there is a gray area specifically in the AST-matching-based bug > finding, which unfortunately leads to duplication of effort. However, we > currently cannot ban/move all AST-based checks out of the analyzer. The main > problem I see with restricting the AST-based analysis to clang-tidy is impact > on the end users. While clang-tidy does export the clang static analyzer > results, the reverse does not hold. There are many end users who do not use > clang-tidy but do use the clang static analyzer (either directly or through > scan-build). Until that is the case, I do not think we should disallow AST > based checks from the analyzer, especially if they come from contributors who > are interested in contributing to this tool. Note that the format in which > the results are presented from these tools is not uniform, which makes > transition more complicated. > > The AST matchers can be used from the analyzer and if the code size becomes a > problem, we could consider moving the analyzer out of the clang codebase. > > Generally, I am not a big fan of the split between the checks based on the > technology used to implement them since it does not lead to a great user > interface - the end users do not think in terms of > path-sensitive/flow-sensitive/AST-matching, they just want to enable specific > functionality such as find bugs or ensure they follow coding guidelines. This > is why I like the existing guidelines with their focus on what clang-tidy is > from user perspective: Agree with it. >> clang-tidy check is a good choice for linter-style checks, checks that are >> related to a certain coding style, checks that address code readability, etc. > > Another thing that could be worth adding here is that clang-tidy finds bugs > that tend to be easy to fix and, in most cases, provide the fixits. (I > believe, this is one of the major strengths of the clang-tidy tool!) > >> Flow-sensitive analyses (that don't need any path-sensitivity) seem to be >> equally suitable for SA and clang-tidy (unless I'm missing something), so I >> don't feel strongly as to where they should go. > > As far as I know, currently, all flow-sensitive analysis are in the Analysis > library in clang. These are analysis for compiler warnings and analysis used > by the static analyzer. I believe this is where future analysis should go as > well, especially, for analysis that could have multiple clients. If clang > code size impact turns out to be a serious problem, we could also have that > library provide APIs that could be used from other tools/checks. (Note, the > analysis could be implemented in the library, but the users of the analysis > (checks) can be elsewhere.) > > Regarding the points about "heuristics" and "flexibility", the analyzer is > full of heuristics and fuzzy API matching. These techniques are very common > in static analysis in general as we are trying to solve undecidable problems > and heuristics is the only way to go. I guess the main problem is that you can't use clang-tidy checks from scan build, where there are some checks like use-after-move, and bunch of misc checks, that would totally fit into what user want from scan build. Repository: rL LLVM https://reviews.llvm.org/D29151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
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 sense in clang-tidy for a few > reasons: > > They usually are easier expressible in terms of AST matchers, and clang-tidy > allows to use AST matchers with less boilerplate. > Clang Static Analyzer is linked into clang, where AST matchers are > undesired, since they tend to blow the size of binary significantly. > It's more consistent to keep all similar analyses together, it simplifies > search for already implemented similar functionality and code reviews. I agree that there is a gray area specifically in the AST-matching-based bug finding, which unfortunately leads to duplication of effort. However, we currently cannot ban/move all AST-based checks out of the analyzer. The main problem I see with restricting the AST-based analysis to clang-tidy is impact on the end users. While clang-tidy does export the clang static analyzer results, the reverse does not hold. There are many end users who do not use clang-tidy but do use the clang static analyzer (either directly or through scan-build). Until that is the case, I do not think we should disallow AST based checks from the analyzer, especially if they come from contributors who are interested in contributing to this tool. Note that the format in which the results are presented from these tools is not uniform, which makes transition more complicated. The AST matchers can be used from the analyzer and if the code size becomes a problem, we could consider moving the analyzer out of the clang codebase. Generally, I am not a big fan of the split between the checks based on the technology used to implement them since it does not lead to a great user interface - the end users do not think in terms of path-sensitive/flow-sensitive/AST-matching, they just want to enable specific functionality such as find bugs or ensure they follow coding guidelines. This is why I like the existing guidelines with their focus on what clang-tidy is from user perspective: > clang-tidy check is a good choice for linter-style checks, checks that are > related to a certain coding style, checks that address code readability, etc. Another thing that could be worth adding here is that clang-tidy finds bugs that tend to be easy to fix and, in most cases, provide the fixits. (I believe, this is one of the major strengths of the clang-tidy tool!) > Flow-sensitive analyses (that don't need any path-sensitivity) seem to be > equally suitable for SA and clang-tidy (unless I'm missing something), so I > don't feel strongly as to where they should go. As far as I know, currently, all flow-sensitive analysis are in the Analysis library in clang. These are analysis for compiler warnings and analysis used by the static analyzer. I believe this is where future analysis should go as well, especially, for analysis that could have multiple clients. If clang code size impact turns out to be a serious problem, we could also have that library provide APIs that could be used from other tools/checks. (Note, the analysis could be implemented in the library, but the users of the analysis (checks) can be elsewhere.) Regarding the points about "heuristics" and "flexibility", the analyzer is full of heuristics and fuzzy API matching. These techniques are very common in static analysis in general as we are trying to solve undecidable problems and heuristics is the only way to go. Repository: rL LLVM https://reviews.llvm.org/D29151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
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 implemented without path-insensitive analysis (including > > flow-sensitive) > > - is checking for language rules (not specific API misuse) > > > > In my view, this should still be the rule going forward because compiler > > warnings are most effective in reaching users. > > > > The Clang Static Analyzer used to be the place for all other diagnostics. > > Most of the checkers it contains rely on path-sensitive analysis. Note that > > one might catch the same bug with flow-sensitive as well as path-sensitive > > analysis. So in some of those cases, we have both warnings as well as > > analyzer checkers finding the same type of user error. However, the > > checkers can catch more bugs since they are path-sensitive. The analyzer > > also has several flow-sensitive/ AST matching checkers. Those checks could > > not have been written as warnings mainly because they check for APIs, which > > are not part of the language. > > > > My understanding is that clang-tidy supports fixits, which the clang > > static analyzer currently does not support. However, there could be other > > benefits to placing not path-sensitive checks there as well. I am not sure. > > I've also heard opinions that it's more of a linter tool, so the user > > expectations could be different. > > > > > Even right now there are clang-tidy checks that finds subset of alpha > > > checks, but probably having lower false positive rate. > > > > Again, alpha checks are unfinished work, so we should not use them as > > examples of checkers that have false positives. Some of them are research > > projects and should probably be deleted. > > > 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: > > | Clang diagnostic: if the check is generic enough, targets code patterns > that most probably are bugs (rather than style or readability issues), can be > implemented effectively and with extremely low false positive rate, it may > make a good Clang diagnostic. | > | Clang static analyzer check: if the check requires some sort of control > flow analysis, it should probably be implemented as a static analyzer check. > >| > | clang-tidy check is a good choice for linter-style checks, checks that are > related to a certain coding style, checks that address code readability, etc. > > | > > There's no doubt path-sensitive checks should go to Static Analyzer, since it > provides all the infrastructure for path-sensitive analyses. Whatever meets > the criteria for a Clang diagnostic should be a diagnostic. Whatever needs > automated fixes (and can be implemented on AST or preprocessor level) should > go to clang-tidy. > > But there's still a large set of analyses that don't clearly fall into one of > the categories above and can be implemented both in Clang Static Analyzer and > in clang-tidy. Currently there are no firm rules about those, only > recommendations on the clang-tidy page (quoted above). We might need to agree > upon some reasonable guidelines, though. > > For example, IMO, AST-based analyses make more sense in clang-tidy for a few > reasons: > > 1. They usually are easier expressible in terms of AST matchers, and > clang-tidy allows to use AST matchers with less boilerplate. > 2. Clang Static Analyzer is linked into clang, where AST matchers are > undesired, since they tend to blow the size of binary significantly. > 3. It's more consistent to keep all similar analyses together, it simplifies > search for already implemented similar functionality and code reviews. > > Flow-sensitive analyses (that don't need any path-sensitivity) seem to be > equally suitable for SA and clang-tidy (unless I'm missing something), so I > don't feel strongly as to where they should go. > > WDYT? This sounds resonable. I think there are also other factors like: - heuristics: clang-tidy tends to look for bugs based on some heuristics. E.g use-after-move consider methods as reinitializing based on method names (like clear). This is approach works great if the heuristic is good enough to not have many false positives, but generally it will have some false positives and false negatives. - flexibility: The other problem is that check can be flexible, for example to work on every vector-like container. I think static analyzer tends to be more conservative about bugs it wants to find. E.g we know how std::vector works, so the
[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
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-sensitive) > - is checking for language rules (not specific API misuse) > > In my view, this should still be the rule going forward because compiler > warnings are most effective in reaching users. > > The Clang Static Analyzer used to be the place for all other diagnostics. > Most of the checkers it contains rely on path-sensitive analysis. Note that > one might catch the same bug with flow-sensitive as well as path-sensitive > analysis. So in some of those cases, we have both warnings as well as > analyzer checkers finding the same type of user error. However, the checkers > can catch more bugs since they are path-sensitive. The analyzer also has > several flow-sensitive/ AST matching checkers. Those checks could not have > been written as warnings mainly because they check for APIs, which are not > part of the language. > > My understanding is that clang-tidy supports fixits, which the clang static > analyzer currently does not support. However, there could be other benefits > to placing not path-sensitive checks there as well. I am not sure. I've also > heard opinions that it's more of a linter tool, so the user expectations > could be different. > > > Even right now there are clang-tidy checks that finds subset of alpha > > checks, but probably having lower false positive rate. > > Again, alpha checks are unfinished work, so we should not use them as > examples of checkers that have false positives. Some of them are research > projects and should probably be deleted. 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: | Clang diagnostic: if the check is generic enough, targets code patterns that most probably are bugs (rather than style or readability issues), can be implemented effectively and with extremely low false positive rate, it may make a good Clang diagnostic. | | Clang static analyzer check: if the check requires some sort of control flow analysis, it should probably be implemented as a static analyzer check. | | clang-tidy check is a good choice for linter-style checks, checks that are related to a certain coding style, checks that address code readability, etc. | There's no doubt path-sensitive checks should go to Static Analyzer, since it provides all the infrastructure for path-sensitive analyses. Whatever meets the criteria for a Clang diagnostic should be a diagnostic. Whatever needs automated fixes (and can be implemented on AST or preprocessor level) should go to clang-tidy. But there's still a large set of analyses that don't clearly fall into one of the categories above and can be implemented both in Clang Static Analyzer and in clang-tidy. Currently there are no firm rules about those, only recommendations on the clang-tidy page (quoted above). We might need to agree upon some reasonable guidelines, though. For example, IMO, AST-based analyses make more sense in clang-tidy for a few reasons: 1. They usually are easier expressible in terms of AST matchers, and clang-tidy allows to use AST matchers with less boilerplate. 2. Clang Static Analyzer is linked into clang, where AST matchers are undesired, since they tend to blow the size of binary significantly. 3. It's more consistent to keep all similar analyses together, it simplifies search for already implemented similar functionality and code reviews. Flow-sensitive analyses (that don't need any path-sensitivity) seem to be equally suitable for SA and clang-tidy (unless I'm missing something), so I don't feel strongly as to where they should go. WDYT? Repository: rL LLVM https://reviews.llvm.org/D29151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
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 my view, this should still be the rule going forward because compiler warnings are most effective in reaching users. The Clang Static Analyzer used to be the place for all other diagnostics. Most of the checkers it contains rely on path-sensitive analysis. Note that one might catch the same bug with flow-sensitive as well as path-sensitive analysis. So in some of those cases, we have both warnings as well as analyzer checkers finding the same type of user error. However, the checkers can catch more bugs since they are path-sensitive. The analyzer also has several flow-sensitive/ AST matching checkers. Those checks could not have been written as warnings mainly because they check for APIs, which are not part of the language. My understanding is that clang-tidy supports fixits, which the clang static analyzer currently does not support. However, there could be other benefits to placing not path-sensitive checks there as well. I am not sure. I've also heard opinions that it's more of a linter tool, so the user expectations could be different. > Even right now there are clang-tidy checks that finds subset of alpha checks, > but probably having lower false positive rate. Again, alpha checks are unfinished work, so we should not use them as examples of checkers that have false positives. Some of them are research projects and should probably be deleted. Repository: rL LLVM https://reviews.llvm.org/D29151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
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, that is doing similar thing as uninitialized > values analysis in clang. I agree with this and could add that we also need similar guidelines for Clang diagnostics too. Some of Clang-tidy and Static Analyzer checks are more suited for Clang diagnostics (unused constructs, deadcode.DeadStores, etc). It will be reasonable if we have discussions to decide better place for particular check instead of just swallowing patches. Repository: rL LLVM https://reviews.llvm.org/D29151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
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 analysis (such as liveness) > now live in lib/Analysis/, which is used by both Sema and the Clang Static > Analyzer. Both path-sensitive and flow-sensitive analysis are based on > clang's CFG. I do not know of clang-tidy uses CFG or lib/Analysis/. > > Here are the wikipedia definitions of sensitivity I am referring to: > //- A flow-sensitive analysis takes into account the order of statements in > a program. For example, a flow-insensitive pointer alias analysis may > determine "variables x and y may refer to the same location", while a > flow-sensitive analysis may determine "after statement 20, variables x and y > may refer to the same location". > > - A path-sensitive analysis computes different pieces of analysis information > dependent on the predicates at conditional branch instructions. For instance, > if a branch contains a condition x>0, then on the fall-through path, the > analysis would assume that x<=0 and on the target of the branch it would > assume that indeed x>0 holds. // > > There is work underway in the analyzer for adding IteratorPastEnd checker. > The first commit was rather large and has been reviewed here > https://reviews.llvm.org/D25660. That checker is very close to be moved out > of alpha. Moving it out of alpha is pending evaluation on real codebases to > ensure that the false positive rates are low and enhancements to error > reporting. > > > Other problem is that it would be probably a part > > of one of the alpha checks, that are not developed and I don't know if > > they will ever be fully supported. > > There seems to be a misconception about what the alpha checkers are. All > checkers start in alpha package to allow in-tree incremental development. > Once the checkers are complete, they should move out of alpha. The criteria > is that the code should pass the review, the checker needs to have low false > positive rates, finally, the error reports should be of good quality (some > checks greatly benefit from extra path hints that can be implemented with a > visitor). These are motivated by providing a good user experience to end > users. (We do have several checkers that are "stuck in alpha". A lot of them > are abandoned experiments that just need to be deleted; others could probably > be made production quality with some extra effort.) 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, that is doing similar thing as uninitialized values analysis in clang. Repository: rL LLVM https://reviews.llvm.org/D29151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
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 by both Sema and the Clang Static Analyzer. Both path-sensitive and flow-sensitive analysis are based on clang's CFG. I do not know of clang-tidy uses CFG or lib/Analysis/. Here are the wikipedia definitions of sensitivity I am referring to: //- A flow-sensitive analysis takes into account the order of statements in a program. For example, a flow-insensitive pointer alias analysis may determine "variables x and y may refer to the same location", while a flow-sensitive analysis may determine "after statement 20, variables x and y may refer to the same location". - A path-sensitive analysis computes different pieces of analysis information dependent on the predicates at conditional branch instructions. For instance, if a branch contains a condition x>0, then on the fall-through path, the analysis would assume that x<=0 and on the target of the branch it would assume that indeed x>0 holds. // There is work underway in the analyzer for adding IteratorPastEnd checker. The first commit was rather large and has been reviewed here https://reviews.llvm.org/D25660. That checker is very close to be moved out of alpha. Moving it out of alpha is pending evaluation on real codebases to ensure that the false positive rates are low and enhancements to error reporting. > Other problem is that it would be probably a part > of one of the alpha checks, that are not developed and I don't know if they > will ever be fully supported. There seems to be a misconception about what the alpha checkers are. All checkers start in alpha package to allow in-tree incremental development. Once the checkers are complete, they should move out of alpha. The criteria is that the code should pass the review, the checker needs to have low false positive rates, finally, the error reports should be of good quality (some checks greatly benefit from extra path hints that can be implemented with a visitor). These are motivated by providing a good user experience to end users. (We do have several checkers that are "stuck in alpha". A lot of them are abandoned experiments that just need to be deleted; others could probably be made production quality with some extra effort.) Repository: rL LLVM https://reviews.llvm.org/D29151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
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 implement this there. Other problem is > that it would be probably a part > of one of the alpha checks, that are not developed and I don't know if they > will ever be fully supported. But it'll be necessary to re-implement part of Static Analyzer functionality in Clang-tidy. Will it be generic enough to be used for similar purposes? It'll be good idea to find out Static Analyzer release criteria. Repository: rL LLVM https://reviews.llvm.org/D29151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
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 one of the alpha checks, that are not developed and I don't know if they will ever be fully supported. Comment at: clang-tidy/misc/InvalidatedIteratorsCheck.h:35 + + // Returns the clang-tidy matcher binding to the possibly invalidating uses + // of the container (either invoking a dangerous member call or calling drop clang-tidy. It's cleaner Comment at: clang-tidy/misc/InvalidatedIteratorsCheck.h:37 + // of the container (either invoking a dangerous member call or calling + // another function with a non-const reference.) + ExprMatcherType getModifyingMatcher(const VarDecl *VectorDecl); ). Repository: rL LLVM https://reviews.llvm.org/D29151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
Prazek added inline comments. Comment at: clang-tidy/misc/InvalidatedIteratorsCheck.cpp:61 + cxxMemberCallExpr(has(memberExpr(hasDeclaration(cxxMethodDecl(hasAnyName( + "push_back", "emplace_back", "clear", + "insert", "emplace"))), Please tests for all of this functions Comment at: clang-tidy/misc/InvalidatedIteratorsCheck.cpp:92 + const auto InsertResult = + CanFuncInvalidateMemo.insert(std::make_pair(MemoTuple, false)); + assert(InsertResult.second); .insert({MemoTuple, false}) Comment at: clang-tidy/misc/InvalidatedIteratorsCheck.cpp:99 +// body; it possibly invalidates our iterators. +return (MemoIter->second = true); + } I guess this might be to optimistic assumption. Normally we should be pesimistic to not introduce false positives. I will run thi check on llvm code base with SmallVector instead of std::vector and will see. Comment at: clang-tidy/misc/InvalidatedIteratorsCheck.cpp:175-179 + const std::unique_ptr TheCFG = + CFG::buildCFG(nullptr, FuncBody, Result.Context, Options); + const std::unique_ptr BlockMap( + new StmtToBlockMap(TheCFG.get(), Result.Context)); + const std::unique_ptr Sequence( const auto BlockMap = std::make_unique(TheCFG.get(), Result.Context) etc. Comment at: clang-tidy/misc/InvalidatedIteratorsCheck.cpp:182-184 + if (!Block) { +return; + } remove extra braces Comment at: docs/clang-tidy/checks/misc-invalidated-iterators.rst:18 + vec.push_back(2017); + ref++; // Incorrect - 'ref' might have been invalidated at 'push_back'. + suggest changing it to ref = 42; When first reading I thought that it is iterator Comment at: test/clang-tidy/misc-invalidated-iterators.cpp:11-17 + class iterator { +Type *data; + + public: +iterator(Type *ptr) : data(ptr) {} +Type *() { return *data; } + }; iterator in vector is just raw pointer, so I guess you can replace it with using iterator = Type*; Comment at: test/clang-tidy/misc-invalidated-iterators.cpp:29 +} + +// Correct std::vector use. Few testcases: - passing vector as pointer - passing vector as copy, it shouldn't care what happens to modyfied vector. - modyfing vector inside loop - does it finds it? like: for (auto : vec) { vec.push_back(42); ref = 42; } Comment at: test/clang-tidy/misc-invalidated-iterators.cpp:125 + // CHECK-MESSAGES: :[[@LINE-4]]:5: note: possible place of invalidation +} add new line to file end Repository: rL LLVM https://reviews.llvm.org/D29151 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.
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/cgi-bin/mailman/listinfo/cfe-commits