whisperity added a comment. Minor comments in-line, looking good on first glance.
I'm not sure if we discussed, has this checker been tried on some in-the-wild examples? To see if there are some real findings or crashes? There is a good selection of projects here in this tool: https://github.com/Xazax-hun/csa-testbench. ================ Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:46 +}; +} + ---------------- Szelethus wrote: > ` // end of anonymous namespace` This comment can be marked as //Done//. ================ Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:52 + llvm::raw_string_ostream OS(Diagnostics); + OS << "Sorting pointer-like keys can result in non-deterministic ordering"; + ---------------- I generally have a concern about calling these thing "keys". How did you mean this? The file's top comment says //elements//. ================ Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:81-87 + callsName("std::is_sorted"), + callsName("std::nth_element"), + callsName("std::partial_sort"), + callsName("std::sort"), + callsName("std::stable_sort"), + callsName("std::partition"), + callsName("std::stable_partition") ---------------- Please order this listing. ================ Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:102 + auto Matches = match(MatcherM, *D, AM.getASTContext()); + for (BoundNodes Match : Matches) + emitDiagnostics(Match, D, BR, AM, this); ---------------- `emitDiagnostics` takes a `const T&`. To prevent unnecessary copy operations in this iteration, you should also use `const BoundNode &Match`. https://reviews.llvm.org/D50488 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
