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<CFG> TheCFG = + CFG::buildCFG(nullptr, FuncBody, Result.Context, Options); + const std::unique_ptr<StmtToBlockMap> BlockMap( + new StmtToBlockMap(TheCFG.get(), Result.Context)); + const std::unique_ptr<ExprSequence> Sequence( ---------------- const auto BlockMap = std::make_unique<StmtToBlockMap>(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 &operator*() { 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 &ref : 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