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

Manuel Klimek via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 14 04:36:52 PDT 2015


klimek added inline comments.

================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:444-448
@@ +443,7 @@
+  };
+  // Keep track of the different coverage situations that have been spotted
+  // during the process: Coverage[Covered][Empty] will tell if there exists any
+  // range that is covered by the first set but not by the second one,
+  // Coverage[Covered][Covered] will tell if there is a range covered by both
+  // sets, etc.
+  bool Coverage[2][2] = {{false, false}, {false, false}};
----------------
I'd just call out the 3 cases, like I suggested in my previous comment ("etc" sounds for me like there's more than 1 case left ;)

================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:461
@@ +460,3 @@
+
+  // If they never appeared at the same position, there is no overlapping.
+  if (!Coverage[Covered][Covered])
----------------
I'd say:
// If both sets never cover the same range, there is no overlap.

================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:472-475
@@ +471,6 @@
+  // first one, but not the other way around.
+  if (Coverage[Empty][Covered])
+    return OK_FirstInsideSecond;
+
+  return OK_SecondInsideFirst;
+}
----------------
I'd make that:
return Coverage[Empty][Covered] ? OK_FirstInsideSecond : OK_SecondInsideFirst;
(and adapt the comment)


http://reviews.llvm.org/D13516





More information about the cfe-commits mailing list