Skip to content

Commit bbc2dde

Browse files
committed
[clang-tidy] Handled insertion only fixits when determining conflicts.
Handle insertion fix-its when removing incompatible errors by introducting a new EventType `ET_Insert` This has lower prioirty than End events, but higher than begin. Idea being If an insert is at the same place as a begin event, the insert should be processed first to reduce unnecessary conflicts. Likewise if its at the same place as an end event, process the end event first for the same reason. This also fixes https://bugs.llvm.org/show_bug.cgi?id=46511. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D82898
1 parent 9ad7c98 commit bbc2dde

File tree

2 files changed

+48
-18
lines changed

2 files changed

+48
-18
lines changed

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "llvm/ADT/STLExtras.h"
3232
#include "llvm/ADT/SmallString.h"
3333
#include "llvm/ADT/StringMap.h"
34+
#include "llvm/Support/ErrorHandling.h"
3435
#include "llvm/Support/FormatVariadic.h"
3536
#include "llvm/Support/Regex.h"
3637
#include <tuple>
@@ -590,6 +591,7 @@ void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() {
590591
// An event can be either the begin or the end of an interval.
591592
enum EventType {
592593
ET_Begin = 1,
594+
ET_Insert = 0,
593595
ET_End = -1,
594596
};
595597

@@ -621,10 +623,17 @@ void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() {
621623
// one will be processed before, disallowing the second one, and the
622624
// end point of the first one will also be processed before,
623625
// disallowing the first one.
624-
if (Type == ET_Begin)
626+
switch (Type) {
627+
case ET_Begin:
625628
Priority = std::make_tuple(Begin, Type, -End, -ErrorSize, ErrorId);
626-
else
629+
break;
630+
case ET_Insert:
631+
Priority = std::make_tuple(Begin, Type, -End, ErrorSize, ErrorId);
632+
break;
633+
case ET_End:
627634
Priority = std::make_tuple(End, Type, -Begin, ErrorSize, ErrorId);
635+
break;
636+
}
628637
}
629638

630639
bool operator<(const Event &Other) const {
@@ -662,19 +671,19 @@ void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() {
662671
}
663672

664673
// Build events from error intervals.
665-
std::map<std::string, std::vector<Event>> FileEvents;
674+
llvm::StringMap<std::vector<Event>> FileEvents;
666675
for (unsigned I = 0; I < ErrorFixes.size(); ++I) {
667676
for (const auto &FileAndReplace : *ErrorFixes[I].second) {
668677
for (const auto &Replace : FileAndReplace.second) {
669678
unsigned Begin = Replace.getOffset();
670679
unsigned End = Begin + Replace.getLength();
671-
const std::string &FilePath = std::string(Replace.getFilePath());
672-
// FIXME: Handle empty intervals, such as those from insertions.
673-
if (Begin == End)
674-
continue;
675-
auto &Events = FileEvents[FilePath];
676-
Events.emplace_back(Begin, End, Event::ET_Begin, I, Sizes[I]);
677-
Events.emplace_back(Begin, End, Event::ET_End, I, Sizes[I]);
680+
auto &Events = FileEvents[Replace.getFilePath()];
681+
if (Begin == End) {
682+
Events.emplace_back(Begin, End, Event::ET_Insert, I, Sizes[I]);
683+
} else {
684+
Events.emplace_back(Begin, End, Event::ET_Begin, I, Sizes[I]);
685+
Events.emplace_back(Begin, End, Event::ET_End, I, Sizes[I]);
686+
}
678687
}
679688
}
680689
}
@@ -686,14 +695,20 @@ void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() {
686695
llvm::sort(Events);
687696
int OpenIntervals = 0;
688697
for (const auto &Event : Events) {
689-
if (Event.Type == Event::ET_End)
690-
--OpenIntervals;
691-
// This has to be checked after removing the interval from the count if it
692-
// is an end event, or before adding it if it is a begin event.
693-
if (OpenIntervals != 0)
694-
Apply[Event.ErrorId] = false;
695-
if (Event.Type == Event::ET_Begin)
696-
++OpenIntervals;
698+
switch (Event.Type) {
699+
case Event::ET_Begin:
700+
if (OpenIntervals++ != 0)
701+
Apply[Event.ErrorId] = false;
702+
break;
703+
case Event::ET_Insert:
704+
if (OpenIntervals != 0)
705+
Apply[Event.ErrorId] = false;
706+
break;
707+
case Event::ET_End:
708+
if (--OpenIntervals != 0)
709+
Apply[Event.ErrorId] = false;
710+
break;
711+
}
697712
}
698713
assert(OpenIntervals == 0 && "Amount of begin/end points doesn't match");
699714
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables,readability-isolate-declaration %t
2+
3+
void foo() {
4+
int A, B, C;
5+
// CHECK-MESSAGES-DAG: :[[@LINE-1]]:7: warning: variable 'A' is not initialized
6+
// CHECK-MESSAGES-DAG: :[[@LINE-2]]:10: warning: variable 'B' is not initialized
7+
// CHECK-MESSAGES-DAG: :[[@LINE-3]]:13: warning: variable 'C' is not initialized
8+
// CHECK-MESSAGES-DAG: :[[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
9+
10+
// Only the isolate declarations fix-it should be applied
11+
12+
// CHECK-FIXES: int A;
13+
// CHECK-FIXES-NEXT: int B;
14+
// CHECK-FIXES-NEXT: int C;
15+
}

0 commit comments

Comments
 (0)