Merge pull request apple#69783 from ahoppen/ahoppen/rename-cleanup-1

Remove two pImpl abstraction layers in rename
slavapestov · Nov 13, 2023 · b598baf · b598baf
2 parents 3d89295 + e6deb0b
commit b598baf
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 95 deletions.
diff --git a/include/swift/Refactoring/Refactoring.h b/include/swift/Refactoring/Refactoring.h
@@ -100,13 +100,11 @@ class FindRenameRangesConsumer {
 };
 
 class FindRenameRangesAnnotatingConsumer : public FindRenameRangesConsumer {
-  struct Implementation;
-  Implementation &Impl;
+  std::unique_ptr<SourceEditConsumer> pRewriter;
 
 public:
   FindRenameRangesAnnotatingConsumer(SourceManager &SM, unsigned BufferId,
                                      llvm::raw_ostream &OS);
-  ~FindRenameRangesAnnotatingConsumer();
   void accept(SourceManager &SM, RegionType RegionType,
               ArrayRef<RenameRangeDetail> Ranges) override;
 };

diff --git a/lib/Refactoring/FindRenameRangesAnnotatingConsumer.cpp b/lib/Refactoring/FindRenameRangesAnnotatingConsumer.cpp
@@ -14,62 +14,50 @@
 
 using namespace swift::refactoring;
 
-struct swift::ide::FindRenameRangesAnnotatingConsumer::Implementation {
-  std::unique_ptr<SourceEditConsumer> pRewriter;
-  Implementation(SourceManager &SM, unsigned BufferId, raw_ostream &OS)
-      : pRewriter(new SourceEditOutputConsumer(SM, BufferId, OS)) {}
-  static StringRef tag(RefactoringRangeKind Kind) {
-    switch (Kind) {
-    case RefactoringRangeKind::BaseName:
-      return "base";
-    case RefactoringRangeKind::KeywordBaseName:
-      return "keywordBase";
-    case RefactoringRangeKind::ParameterName:
-      return "param";
-    case RefactoringRangeKind::NoncollapsibleParameterName:
-      return "noncollapsibleparam";
-    case RefactoringRangeKind::DeclArgumentLabel:
-      return "arglabel";
-    case RefactoringRangeKind::CallArgumentLabel:
-      return "callarg";
-    case RefactoringRangeKind::CallArgumentColon:
-      return "callcolon";
-    case RefactoringRangeKind::CallArgumentCombined:
-      return "callcombo";
-    case RefactoringRangeKind::SelectorArgumentLabel:
-      return "sel";
-    }
-    llvm_unreachable("unhandled kind");
+static StringRef tag(RefactoringRangeKind Kind) {
+  switch (Kind) {
+  case RefactoringRangeKind::BaseName:
+    return "base";
+  case RefactoringRangeKind::KeywordBaseName:
+    return "keywordBase";
+  case RefactoringRangeKind::ParameterName:
+    return "param";
+  case RefactoringRangeKind::NoncollapsibleParameterName:
+    return "noncollapsibleparam";
+  case RefactoringRangeKind::DeclArgumentLabel:
+    return "arglabel";
+  case RefactoringRangeKind::CallArgumentLabel:
+    return "callarg";
+  case RefactoringRangeKind::CallArgumentColon:
+    return "callcolon";
+  case RefactoringRangeKind::CallArgumentCombined:
+    return "callcombo";
+  case RefactoringRangeKind::SelectorArgumentLabel:
+    return "sel";
   }
-  void accept(SourceManager &SM, const RenameRangeDetail &Range) {
-    std::string NewText;
-    llvm::raw_string_ostream OS(NewText);
-    StringRef Tag = tag(Range.RangeKind);
-    OS << "<" << Tag;
-    if (Range.Index.has_value())
-      OS << " index=" << *Range.Index;
-    OS << ">" << Range.Range.str() << "</" << Tag << ">";
-    pRewriter->accept(SM, {/*Path=*/{}, Range.Range, /*BufferName=*/{},
-                           OS.str(), /*RegionsWorthNote=*/{}});
-  }
-};
+  llvm_unreachable("unhandled kind");
+}
 
 swift::ide::FindRenameRangesAnnotatingConsumer::
     FindRenameRangesAnnotatingConsumer(SourceManager &SM, unsigned BufferId,
                                        raw_ostream &OS)
-    : Impl(*new Implementation(SM, BufferId, OS)) {}
-
-swift::ide::FindRenameRangesAnnotatingConsumer::
-    ~FindRenameRangesAnnotatingConsumer() {
-  delete &Impl;
-}
+    : pRewriter(new SourceEditOutputConsumer(SM, BufferId, OS)) {}
 
 void swift::ide::FindRenameRangesAnnotatingConsumer::accept(
     SourceManager &SM, RegionType RegionType,
     ArrayRef<RenameRangeDetail> Ranges) {
   if (RegionType == RegionType::Mismatch || RegionType == RegionType::Unmatched)
     return;
   for (const auto &Range : Ranges) {
-    Impl.accept(SM, Range);
+    std::string NewText;
+    llvm::raw_string_ostream OS(NewText);
+    StringRef Tag = tag(Range.RangeKind);
+    OS << "<" << Tag;
+    if (Range.Index.has_value())
+      OS << " index=" << *Range.Index;
+    OS << ">" << Range.Range.str() << "</" << Tag << ">";
+    Replacement Repl{/*Path=*/{}, Range.Range, /*BufferName=*/{}, OS.str(),
+                     /*RegionsWorthNote=*/{}};
+    pRewriter->accept(SM, Repl);
   }
 }
diff --git a/tools/SourceKit/lib/SwiftLang/SwiftDocSupport.cpp b/tools/SourceKit/lib/SwiftLang/SwiftDocSupport.cpp
@@ -1344,60 +1344,41 @@ void RequestRefactoringEditConsumer::handleDiagnostic(
   Impl.DiagConsumer.handleDiagnostic(SM, Info);
 }
 
-class RequestRenameRangeConsumer::Implementation {
-  CategorizedRenameRangesReceiver Receiver;
-  std::string ErrBuffer;
-  llvm::raw_string_ostream OS;
-  std::vector<CategorizedRenameRanges> CategorizedRanges;
-
-public:
-  PrintingDiagnosticConsumer DiagConsumer;
-
-public:
-  Implementation(CategorizedRenameRangesReceiver Receiver)
-      : Receiver(Receiver), OS(ErrBuffer), DiagConsumer(OS) {}
-
-  ~Implementation() {
-    if (DiagConsumer.didErrorOccur()) {
-      Receiver(RequestResult<ArrayRef<CategorizedRenameRanges>>::fromError(OS.str()));
-      return;
-    }
-    Receiver(RequestResult<ArrayRef<CategorizedRenameRanges>>::fromResult(CategorizedRanges));
-  }
-
-  void accept(SourceManager &SM, RegionType RegionType,
-              ArrayRef<ide::RenameRangeDetail> Ranges) {
-    CategorizedRenameRanges Results;
-    Results.Category = SwiftLangSupport::getUIDForRegionType(RegionType);
-    for (const auto &R : Ranges) {
-      SourceKit::RenameRangeDetail Result;
-      std::tie(Result.StartLine, Result.StartColumn) =
-          SM.getLineAndColumnInBuffer(R.Range.getStart());
-      std::tie(Result.EndLine, Result.EndColumn) =
-          SM.getLineAndColumnInBuffer(R.Range.getEnd());
-      Result.ArgIndex = R.Index;
-      Result.Kind =
-          SwiftLangSupport::getUIDForRefactoringRangeKind(R.RangeKind);
-      Results.Ranges.push_back(std::move(Result));
-    }
-    CategorizedRanges.push_back(std::move(Results));
-  }
-};
-
 RequestRenameRangeConsumer::RequestRenameRangeConsumer(
     CategorizedRenameRangesReceiver Receiver)
-    : Impl(*new Implementation(Receiver)) {}
-RequestRenameRangeConsumer::~RequestRenameRangeConsumer() { delete &Impl; }
+    : Receiver(Receiver), OS(ErrBuffer), DiagConsumer(OS) {}
+
+RequestRenameRangeConsumer::~RequestRenameRangeConsumer() {
+  if (DiagConsumer.didErrorOccur()) {
+    Receiver(
+        RequestResult<ArrayRef<CategorizedRenameRanges>>::fromError(OS.str()));
+    return;
+  }
+  Receiver(RequestResult<ArrayRef<CategorizedRenameRanges>>::fromResult(
+      CategorizedRanges));
+}
 
 void RequestRenameRangeConsumer::accept(
     SourceManager &SM, RegionType RegionType,
     ArrayRef<ide::RenameRangeDetail> Ranges) {
-  Impl.accept(SM, RegionType, Ranges);
+  CategorizedRenameRanges Results;
+  Results.Category = SwiftLangSupport::getUIDForRegionType(RegionType);
+  for (const auto &R : Ranges) {
+    SourceKit::RenameRangeDetail Result;
+    std::tie(Result.StartLine, Result.StartColumn) =
+        SM.getLineAndColumnInBuffer(R.Range.getStart());
+    std::tie(Result.EndLine, Result.EndColumn) =
+        SM.getLineAndColumnInBuffer(R.Range.getEnd());
+    Result.ArgIndex = R.Index;
+    Result.Kind = SwiftLangSupport::getUIDForRefactoringRangeKind(R.RangeKind);
+    Results.Ranges.push_back(std::move(Result));
+  }
+  CategorizedRanges.push_back(std::move(Results));
 }
 
 void RequestRenameRangeConsumer::handleDiagnostic(SourceManager &SM,
                                                   const DiagnosticInfo &Info) {
-  Impl.DiagConsumer.handleDiagnostic(SM, Info);
+  DiagConsumer.handleDiagnostic(SM, Info);
 }
 
 static NameUsage getNameUsage(RenameType Type) {

diff --git a/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h b/tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h
@@ -24,12 +24,13 @@
 #include "swift/AST/DiagnosticConsumer.h"
 #include "swift/AST/PluginRegistry.h"
 #include "swift/Basic/ThreadSafeRefCounted.h"
+#include "swift/Frontend/PrintingDiagnosticConsumer.h"
 #include "swift/IDE/CancellableResult.h"
 #include "swift/IDE/Indenting.h"
-#include "swift/Refactoring/Refactoring.h"
 #include "swift/IDETool/CompileInstance.h"
 #include "swift/IDETool/IDEInspectionInstance.h"
 #include "swift/Index/IndexSymbol.h"
+#include "swift/Refactoring/Refactoring.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Mutex.h"
@@ -276,8 +277,12 @@ class RequestRefactoringEditConsumer: public swift::ide::SourceEditConsumer,
 
 class RequestRenameRangeConsumer : public swift::ide::FindRenameRangesConsumer,
                                    public swift::DiagnosticConsumer {
-  class Implementation;
-  Implementation &Impl;
+  CategorizedRenameRangesReceiver Receiver;
+  std::string ErrBuffer;
+  llvm::raw_string_ostream OS;
+  std::vector<CategorizedRenameRanges> CategorizedRanges;
+
+  swift::PrintingDiagnosticConsumer DiagConsumer;
 
 public:
   RequestRenameRangeConsumer(CategorizedRenameRangesReceiver Receiver);