[clangd] Fix crash in applyTweak, remove TweakID alias.

Strings are complicated, giving them opaque names makes us forget
they're complicated.

llvm-svn: 352837

diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 7221b38..6857b42 100644 (file)
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -354,10 +354,10 @@ void ClangdServer::enumerateTweaks(PathRef File, Range Sel,
                            Bind(Action, std::move(CB), File.str()));
 }
 
-void ClangdServer::applyTweak(PathRef File, Range Sel, TweakID ID,
+void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
                               Callback<tooling::Replacements> CB) {
-  auto Action = [ID, Sel](decltype(CB) CB, std::string File,
-                          Expected<InputsAndAST> InpAST) {
+  auto Action = [Sel](decltype(CB) CB, std::string File, std::string TweakID,
+                      Expected<InputsAndAST> InpAST) {
     if (!InpAST)
       return CB(InpAST.takeError());
 
@@ -369,14 +369,15 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, TweakID ID,
     Tweak::Selection Inputs = {InpAST->Inputs.Contents, InpAST->AST,
                                *CursorLoc};
 
-    auto A = prepareTweak(ID, Inputs);
+    auto A = prepareTweak(TweakID, Inputs);
     if (!A)
       return CB(A.takeError());
     // FIXME: run formatter on top of resulting replacements.
     return CB((*A)->apply(Inputs));
   };
-  WorkScheduler.runWithAST("ApplyTweak", File,
-                           Bind(Action, std::move(CB), File.str()));
+  WorkScheduler.runWithAST(
+      "ApplyTweak", File,
+      Bind(Action, std::move(CB), File.str(), TweakID.str()));
 }
 
 void ClangdServer::dumpAST(PathRef File,

diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index cc9f713..f53718f 100644 (file)
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -214,7 +214,7 @@ public:
               Callback<std::vector<tooling::Replacement>> CB);
 
   struct TweakRef {
-    TweakID ID;        /// ID to pass for applyTweak.
+    std::string ID;    /// ID to pass for applyTweak.
     std::string Title; /// A single-line message to show in the UI.
   };
   /// Enumerate the code tweaks available to the user at a specified point.
@@ -222,7 +222,7 @@ public:
                        Callback<std::vector<TweakRef>> CB);
 
   /// Apply the code tweak with a specified \p ID.
-  void applyTweak(PathRef File, Range Sel, TweakID ID,
+  void applyTweak(PathRef File, Range Sel, StringRef ID,
                   Callback<tooling::Replacements> CB);
 
   /// Only for testing purposes.

diff --git a/clang-tools-extra/clangd/refactor/Tweak.cpp b/clang-tools-extra/clangd/refactor/Tweak.cpp
index de6020c..02a2244 100644 (file)
--- a/clang-tools-extra/clangd/refactor/Tweak.cpp
+++ b/clang-tools-extra/clangd/refactor/Tweak.cpp
@@ -55,7 +55,7 @@ std::vector<std::unique_ptr<Tweak>> prepareTweaks(const Tweak::Selection &S) {
   return Available;
 }
 
-llvm::Expected<std::unique_ptr<Tweak>> prepareTweak(TweakID ID,
+llvm::Expected<std::unique_ptr<Tweak>> prepareTweak(StringRef ID,
                                                     const Tweak::Selection &S) {
   auto It = llvm::find_if(
       TweakRegistry::entries(),

diff --git a/clang-tools-extra/clangd/refactor/Tweak.h b/clang-tools-extra/clangd/refactor/Tweak.h
index df00bf7..abd4995 100644 (file)
--- a/clang-tools-extra/clangd/refactor/Tweak.h
+++ b/clang-tools-extra/clangd/refactor/Tweak.h
@@ -27,14 +27,11 @@
 namespace clang {
 namespace clangd {
 
-using TweakID = llvm::StringRef;
-
 /// An interface base for small context-sensitive refactoring actions.
 /// To implement a new tweak use the following pattern in a .cpp file:
 ///   class MyTweak : public Tweak {
 ///   public:
-///     TweakID id() const override final; // definition provided by
-///                                        // REGISTER_TWEAK.
+///     const char* id() const override final; // defined by REGISTER_TWEAK.
 ///     // implement other methods here.
 ///   };
 ///   REGISTER_TWEAK(MyTweak);
@@ -57,7 +54,7 @@ public:
   /// A unique id of the action, it is always equal to the name of the class
   /// defining the Tweak. Definition is provided automatically by
   /// REGISTER_TWEAK.
-  virtual TweakID id() const = 0;
+  virtual const char *id() const = 0;
   /// Run the first stage of the action. The non-None result indicates that the
   /// action is available and should be shown to the user. Returns None if the
   /// action is not available.
@@ -78,9 +75,7 @@ public:
 #define REGISTER_TWEAK(Subclass)                                               \
   ::llvm::Registry<::clang::clangd::Tweak>::Add<Subclass>                      \
       TweakRegistrationFor##Subclass(#Subclass, /*Description=*/"");           \
-  ::clang::clangd::TweakID Subclass::id() const {                              \
-    return llvm::StringLiteral(#Subclass);                                     \
-  }
+  const char *Subclass::id() const { return #Subclass; }
 
 /// Calls prepare() on all tweaks, returning those that can run on the
 /// selection.
@@ -89,7 +84,7 @@ std::vector<std::unique_ptr<Tweak>> prepareTweaks(const Tweak::Selection &S);
 // Calls prepare() on the tweak with a given ID.
 // If prepare() returns false, returns an error.
 // If prepare() returns true, returns the corresponding tweak.
-llvm::Expected<std::unique_ptr<Tweak>> prepareTweak(TweakID ID,
+llvm::Expected<std::unique_ptr<Tweak>> prepareTweak(StringRef TweakID,
                                                     const Tweak::Selection &S);
 
 } // namespace clangd

diff --git a/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp b/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
index 4de498f..2e0f33a 100644 (file)
--- a/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -34,7 +34,7 @@ namespace {
 ///   if (foo) { continue; } else { return 10; }
 class SwapIfBranches : public Tweak {
 public:
-  TweakID id() const override final;
+  const char *id() const override final;
 
   bool prepare(const Selection &Inputs) override;
   Expected<tooling::Replacements> apply(const Selection &Inputs) override;

diff --git a/clang-tools-extra/unittests/clangd/TweakTests.cpp b/clang-tools-extra/unittests/clangd/TweakTests.cpp
index 8306b48..905f06b 100644 (file)
--- a/clang-tools-extra/unittests/clangd/TweakTests.cpp
+++ b/clang-tools-extra/unittests/clangd/TweakTests.cpp
@@ -24,8 +24,6 @@
 using llvm::Failed;
 using llvm::HasValue;
 using llvm::Succeeded;
-using ::testing::IsEmpty;
-using ::testing::Not;
 
 namespace clang {
 namespace clangd {
@@ -43,7 +41,7 @@ std::string markRange(llvm::StringRef Code, Range R) {
       .str();
 }
 
-void checkAvailable(TweakID ID, llvm::StringRef Input, bool Available) {
+void checkAvailable(StringRef ID, llvm::StringRef Input, bool Available) {
   Annotations Code(Input);
   ASSERT_TRUE(0 < Code.points().size() || 0 < Code.ranges().size())
       << "no points of interest specified";
@@ -71,15 +69,15 @@ void checkAvailable(TweakID ID, llvm::StringRef Input, bool Available) {
 }
 
 /// Checks action is available at every point and range marked in \p Input.
-void checkAvailable(TweakID ID, llvm::StringRef Input) {
+void checkAvailable(StringRef ID, llvm::StringRef Input) {
   return checkAvailable(ID, Input, /*Available=*/true);
 }
 
 /// Same as checkAvailable, but checks the action is not available.
-void checkNotAvailable(TweakID ID, llvm::StringRef Input) {
+void checkNotAvailable(StringRef ID, llvm::StringRef Input) {
   return checkAvailable(ID, Input, /*Available=*/false);
 }
-llvm::Expected<std::string> apply(TweakID ID, llvm::StringRef Input) {
+llvm::Expected<std::string> apply(StringRef ID, llvm::StringRef Input) {
   Annotations Code(Input);
   Range SelectionRng;
   if (Code.points().size() != 0) {