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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits