[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 281418.
njames93 marked 2 inline comments as done.
njames93 added a comment.

Remove unreachable after switch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82898/new/

https://reviews.llvm.org/D82898

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables-conflict.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables-conflict.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables-conflict.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables,readability-isolate-declaration %t
+
+void foo() {
+  int A, B, C;
+  // CHECK-MESSAGES-DAG: :[[@LINE-1]]:7: warning: variable 'A' is not initialized
+  // CHECK-MESSAGES-DAG: :[[@LINE-2]]:10: warning: variable 'B' is not initialized
+  // CHECK-MESSAGES-DAG: :[[@LINE-3]]:13: warning: variable 'C' is not initialized
+  // CHECK-MESSAGES-DAG: :[[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+
+  // Only the isolate declarations fix-it should be applied
+
+  //      CHECK-FIXES: int A;
+  // CHECK-FIXES-NEXT: int B;
+  // CHECK-FIXES-NEXT: int C;
+}
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -31,6 +31,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Regex.h"
 #include <tuple>
@@ -590,6 +591,7 @@
     // An event can be either the begin or the end of an interval.
     enum EventType {
       ET_Begin = 1,
+      ET_Insert = 0,
       ET_End = -1,
     };
 
@@ -621,10 +623,17 @@
       //   one will be processed before, disallowing the second one, and the
       //   end point of the first one will also be processed before,
       //   disallowing the first one.
-      if (Type == ET_Begin)
+      switch (Type) {
+      case ET_Begin:
         Priority = std::make_tuple(Begin, Type, -End, -ErrorSize, ErrorId);
-      else
+        break;
+      case ET_Insert:
+        Priority = std::make_tuple(Begin, Type, -End, ErrorSize, ErrorId);
+        break;
+      case ET_End:
         Priority = std::make_tuple(End, Type, -Begin, ErrorSize, ErrorId);
+        break;
+      }
     }
 
     bool operator<(const Event &Other) const {
@@ -662,19 +671,19 @@
   }
 
   // Build events from error intervals.
-  std::map<std::string, std::vector<Event>> FileEvents;
+  llvm::StringMap<std::vector<Event>> FileEvents;
   for (unsigned I = 0; I < ErrorFixes.size(); ++I) {
     for (const auto &FileAndReplace : *ErrorFixes[I].second) {
       for (const auto &Replace : FileAndReplace.second) {
         unsigned Begin = Replace.getOffset();
         unsigned End = Begin + Replace.getLength();
-        const std::string &FilePath = std::string(Replace.getFilePath());
-        // FIXME: Handle empty intervals, such as those from insertions.
-        if (Begin == End)
-          continue;
-        auto &Events = FileEvents[FilePath];
-        Events.emplace_back(Begin, End, Event::ET_Begin, I, Sizes[I]);
-        Events.emplace_back(Begin, End, Event::ET_End, I, Sizes[I]);
+        auto &Events = FileEvents[Replace.getFilePath()];
+        if (Begin == End) {
+          Events.emplace_back(Begin, End, Event::ET_Insert, I, Sizes[I]);
+        } else {
+          Events.emplace_back(Begin, End, Event::ET_Begin, I, Sizes[I]);
+          Events.emplace_back(Begin, End, Event::ET_End, I, Sizes[I]);
+        }
       }
     }
   }
@@ -686,14 +695,20 @@
     llvm::sort(Events);
     int OpenIntervals = 0;
     for (const auto &Event : Events) {
-      if (Event.Type == Event::ET_End)
-        --OpenIntervals;
-      // This has to be checked after removing the interval from the count if it
-      // is an end event, or before adding it if it is a begin event.
-      if (OpenIntervals != 0)
-        Apply[Event.ErrorId] = false;
-      if (Event.Type == Event::ET_Begin)
-        ++OpenIntervals;
+      switch (Event.Type) {
+      case Event::ET_Begin:
+        if (OpenIntervals++ != 0)
+          Apply[Event.ErrorId] = false;
+        break;
+      case Event::ET_Insert:
+        if (OpenIntervals != 0)
+          Apply[Event.ErrorId] = false;
+        break;
+      case Event::ET_End:
+        if (--OpenIntervals != 0)
+          Apply[Event.ErrorId] = false;
+        break;
+      }
     }
     assert(OpenIntervals == 0 && "Amount of begin/end points doesn't match");
   }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits