[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