[PATCH] D13516: Fix overlapping replacements in clang-tidy.

[Manuel Klimek via cfe-commits]

klimek added a comment.

LG in general now, looks to me like we have very few tests though.
My favorite strategy to make sure I have enough tests is to comment out code (or do mutations) as long as the tests still pass. Then write tests that fail with the mutation, then undo the mutation.


================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:492-493
@@ +491,4 @@
+
+  // Build the sets of intervals, and their bounding boxes.
+  unsigned NumErrors = Errors.size();
+  for (unsigned I = 0; I < NumErrors; ++I) {
----------------
I think a comment at the top of this function outlining the full algorithm would be nice (without lots of details).

================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:519-521
@@ +518,5 @@
+  std::vector<bool> Apply(NumErrors, true);
+  // We use a set because we want the functionality of a priority queue but we
+  // also need to iterate over the elements.
+  std::set<std::pair<unsigned, unsigned>> Queue;
+  for (unsigned I = 0; I < Shapes.size(); ++I) {
----------------
Document what goes into the pair - I assumed 'first, second' for a moment, and then was confused.


http://reviews.llvm.org/D13516