[clang-tools-extra] r352837 - [clangd] Fix crash in applyTweak, remove TweakID alias.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 31 21:41:50 PST 2019


Author: sammccall
Date: Thu Jan 31 21:41:50 2019
New Revision: 352837

URL: http://llvm.org/viewvc/llvm-project?rev=352837&view=rev
Log:
[clangd] Fix crash in applyTweak, remove TweakID alias.

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

Modified:
    clang-tools-extra/trunk/clangd/ClangdServer.cpp
    clang-tools-extra/trunk/clangd/ClangdServer.h
    clang-tools-extra/trunk/clangd/refactor/Tweak.cpp
    clang-tools-extra/trunk/clangd/refactor/Tweak.h
    clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp
    clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=352837&r1=352836&r2=352837&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Jan 31 21:41:50 2019
@@ -354,10 +354,10 @@ void ClangdServer::enumerateTweaks(PathR
                            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 Fi
     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,

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=352837&r1=352836&r2=352837&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Thu Jan 31 21:41:50 2019
@@ -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.

Modified: clang-tools-extra/trunk/clangd/refactor/Tweak.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Tweak.cpp?rev=352837&r1=352836&r2=352837&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/refactor/Tweak.cpp (original)
+++ clang-tools-extra/trunk/clangd/refactor/Tweak.cpp Thu Jan 31 21:41:50 2019
@@ -55,7 +55,7 @@ std::vector<std::unique_ptr<Tweak>> prep
   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(),

Modified: clang-tools-extra/trunk/clangd/refactor/Tweak.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Tweak.h?rev=352837&r1=352836&r2=352837&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/refactor/Tweak.h (original)
+++ clang-tools-extra/trunk/clangd/refactor/Tweak.h Thu Jan 31 21:41:50 2019
@@ -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>> prep
 // 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

Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp?rev=352837&r1=352836&r2=352837&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp (original)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp Thu Jan 31 21:41:50 2019
@@ -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;

Modified: clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp?rev=352837&r1=352836&r2=352837&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp Thu Jan 31 21:41:50 2019
@@ -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 Co
       .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::St
 }
 
 /// 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) {




More information about the cfe-commits mailing list