angelgarcia updated this revision to Diff 37569.
angelgarcia added a comment.
Remove unused include, fix typo and rename Count to OpenIntervals.
http://reviews.llvm.org/D13516
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
unittests/clang-tidy/O
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:489
@@ +488,3 @@
+ std::vector Apply(Errors.size(), true);
+ int Count = 0;
+ for (const auto &Event : Events) {
-
angelgarcia updated this revision to Diff 37568.
angelgarcia added a comment.
New algorithm :)
http://reviews.llvm.org/D13516
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
unittests/clang-tidy/OverlappingReplacementsTest.cpp
Index: unittests/c
angelgarcia added a comment.
I implemented this, with the following addition: if several errors share
the same interval, I can still apply the biggest one that was not discarded
during the sweep. This way, the first example would work and in the two
other examples it would just apply a random one,
On Thu, Oct 15, 2015 at 6:29 AM, Angel Garcia
wrote:
> angelgarcia added a comment.
>
> I cannot find a way to make Daniel's idea work with equal intervals:
>
> In this case, fix A can be applied because B is completely contained inside
> it.
> A: [a, b)[c, d)
> B: [a, b)
>
> This time, we should
angelgarcia added a comment.
I cannot find a way to make Daniel's idea work with equal intervals:
In this case, fix A can be applied because B is completely contained inside
it.
A: [a, b)[c, d)
B: [a, b)
This time, we should not apply anyone:
A: [a, b)
B: [a, b)
And here they both have to be di
angelgarcia updated this revision to Diff 37465.
angelgarcia added a comment.
I did several mutations and the only case were a test didn't break was when I
removed the sort, but it turned out that we don't need it. I changed the tests
to apply the checks in both orders to ensure that a test will
angelgarcia added a comment.
That works pretty well (well, identical end points have to be sorted
decreasingly, if I understand correctly). I see a couple of problems
though, like equal intervals, or consecutive. But the algorithm is way more
efficient than what we currently have, I will try to ma
djasper added a comment.
There might be an even easier algorithm:
What if we put all start and end points into a single sorted list. Identical
start points are sorted by decreasing end points and identical end points are
sorted by increasing start points. Now do a single sweep and simply count
angelgarcia added a comment.
I've done a couple of runs for each version, and these are the results (I
have clang-tidy compiled with the option "RelWithDebInfo"):
$ time clang-tidy -checks=* test.cpp -- -std=c++11
Without looking for overlaps.
Suppressed 23463 warnings (23463 in non-user code).
djasper added a subscriber: djasper.
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:495-496
@@ +494,4 @@
+ for (unsigned I = 0; I < NumErrors; ++I) {
+const auto &Error = Errors[I];
+// Skip errors without fixes. They are never going to overlap with anything
+
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.
=
angelgarcia updated this revision to Diff 37344.
angelgarcia added a comment.
Use bounding boxes to reduce complexity.
http://reviews.llvm.org/D13516
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
unittests/clang-tidy/OverlappingReplacementsTest
klimek added inline comments.
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:495-496
@@ +494,4 @@
+ std::vector Apply(NumErrors, true);
+ for (int I = 0; I < NumErrors; ++I) {
+for (int J = I + 1; J < NumErrors; ++J) {
+ OverlappingKind Kind =
I
angelgarcia updated this revision to Diff 37336.
angelgarcia added a comment.
Done.
http://reviews.llvm.org/D13516
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
unittests/clang-tidy/OverlappingReplacementsTest.cpp
Index: unittests/clang-tidy/O
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
angelgarcia updated this revision to Diff 37334.
angelgarcia added a comment.
Add an enum and rename "Sit" to "Coverage" to improve readability.
http://reviews.llvm.org/D13516
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
unittests/clang-tidy/O
klimek added inline comments.
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:438-440
@@ +437,5 @@
+ int Count[2] = {0, 0};
+ // Sit[1][0] will tell if there exists any range that is covered by the
+ // first set but not by the second one, Sit[1][1] will tell if there is
klimek added a comment.
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:438-440
@@ +437,5 @@
+ int Count[2] = {0, 0};
+ // Sit[1][0] will tell if there exists any range that is covered by the
+ // first set but not by the second one, Sit[1][1] will tell if there is a
angelgarcia added a comment.
> These need to be documented.
Done.
> I'd name this Queue instead (reading later I had no idea what this was).
Done.
> Why are you calling this "Sit"?
I didn't even know how to describe this variable without using examples,
and naming it is harder. More or les
angelgarcia updated this revision to Diff 37323.
angelgarcia added a comment.
Fix comments.
http://reviews.llvm.org/D13516
Files:
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
unittests/clang-tidy/OverlappingReplacementsTest.cpp
Index: unittests/clan
klimek added inline comments.
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:418-419
@@ +417,4 @@
+unsigned Pos;
+int Type;
+int ErrorId;
+ };
These need to be documented.
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cp
angelgarcia created this revision.
angelgarcia added reviewers: klimek, bkramer.
angelgarcia added subscribers: alexfh, cfe-commits.
Prevent clang-tidy from applying fixes to errors that overlap with other
errors' fixes, with one exception: if one fix is completely contained inside
another one,
23 matches
Mail list logo
