On 1/30/19 10:41 AM, Ilya Biryukov wrote: > Should be fixed by r352612, let me know if it pops up again. > This was a use after move, probably gcc and clang prefer different > evaluation order, hence the difference in behaviours.
Awesome! Now it passes also when I compile with gcc. Thanks, Mikael > > On Wed, Jan 30, 2019 at 12:12 PM Ilya Biryukov <ibiryu...@google.com > <mailto:ibiryu...@google.com>> wrote: > > Hi Mikael, > > I suspect an undefined value creeps in for protocol capabilities, > given that we have responses in different formats. > I'll investigate, thanks for the report! > > On Wed, Jan 30, 2019 at 12:05 PM Mikael Holmén > <mikael.hol...@ericsson.com <mailto:mikael.hol...@ericsson.com>> wrote: > > Hi Ilya, > > I've no idea why, but when I compile this commit with gcc > (7.4.0) the > test fixits-command.test fails, and the output from clangd is > rather > different from when I compile it with clang (3.6.0). > > So I run > > build-all/bin/clangd -lit-test < > > /data/repo/master/llvm-master/tools/clang/tools/extra/test/clangd/fixits-command.test > > and the first diff that I think is significant is that with gcc > it says > > E[09:44:29.470] error while getting semantic code actions: -32602: > trying to get AST for non-added document > > Before this patch the output from gcc and clang built clangd is > identical. > > Attaching logs with gcc and clang. > > Any idea what the problem is? > > Regards, > Mikael > > On 1/29/19 3:17 PM, Ilya Biryukov via cfe-commits wrote: > > Author: ibiryukov > > Date: Tue Jan 29 06:17:36 2019 > > New Revision: 352494 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=352494&view=rev > > Log: > > [clangd] Interfaces for writing code tweaks > > > > Summary: > > The code tweaks are an implementation of mini-refactorings > exposed > > via the LSP code actions. They run in two stages: > > - Stage 1. Decides whether the action is available to the > user and > > collects all the information required to finish the action. > > Should be cheap, since this will run over all the > actions known to > > clangd on each textDocument/codeAction request from the > client. > > > > - Stage 2. Uses information from stage 1 to produce the > actual edits > > that the code action should perform. This stage can be > expensive and > > will only run if the user chooses to perform the > specified action in > > the UI. > > > > One unfortunate consequence of this change is increased > latency of > > processing the textDocument/codeAction requests, which now > wait for an > > AST. However, we cannot avoid this with what we have > available in the LSP > > today. > > > > Reviewers: kadircet, ioeric, hokein, sammccall > > > > Reviewed By: sammccall > > > > Subscribers: mgrang, mgorny, MaskRay, jkorous, arphaman, > cfe-commits > > > > Differential Revision: https://reviews.llvm.org/D56267 > > > > Added: > > clang-tools-extra/trunk/clangd/refactor/ > > clang-tools-extra/trunk/clangd/refactor/Tweak.cpp > > clang-tools-extra/trunk/clangd/refactor/Tweak.h > > clang-tools-extra/trunk/clangd/refactor/tweaks/ > > > clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt > > clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp > > Modified: > > clang-tools-extra/trunk/clangd/CMakeLists.txt > > clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp > > clang-tools-extra/trunk/clangd/ClangdServer.cpp > > clang-tools-extra/trunk/clangd/ClangdServer.h > > clang-tools-extra/trunk/clangd/Protocol.cpp > > clang-tools-extra/trunk/clangd/Protocol.h > > clang-tools-extra/trunk/clangd/SourceCode.cpp > > clang-tools-extra/trunk/clangd/SourceCode.h > > clang-tools-extra/trunk/clangd/tool/CMakeLists.txt > > clang-tools-extra/trunk/test/clangd/fixits-command.test > > clang-tools-extra/trunk/test/clangd/initialize-params.test > > > > Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=352494&r1=352493&r2=352494&view=diff > > > > ============================================================================== > > --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original) > > +++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue Jan 29 > 06:17:36 2019 > > @@ -71,6 +71,8 @@ add_clang_library(clangDaemon > > index/dex/PostingList.cpp > > index/dex/Trigram.cpp > > > > + refactor/Tweak.cpp > > + > > LINK_LIBS > > clangAST > > clangASTMatchers > > @@ -108,6 +110,7 @@ add_clang_library(clangDaemon > > ${CLANGD_ATOMIC_LIB} > > ) > > > > +add_subdirectory(refactor/tweaks) > > if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE ) > > add_subdirectory(fuzzer) > > endif() > > > > Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=352494&r1=352493&r2=352494&view=diff > > > > ============================================================================== > > --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original) > > +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue > Jan 29 06:17:36 2019 > > @@ -8,11 +8,14 @@ > > > > #include "ClangdLSPServer.h" > > #include "Diagnostics.h" > > +#include "Protocol.h" > > #include "SourceCode.h" > > #include "Trace.h" > > #include "URI.h" > > +#include "clang/Tooling/Core/Replacement.h" > > #include "llvm/ADT/ScopeExit.h" > > #include "llvm/Support/Errc.h" > > +#include "llvm/Support/Error.h" > > #include "llvm/Support/FormatVariadic.h" > > #include "llvm/Support/Path.h" > > #include "llvm/Support/ScopedPrinter.h" > > @@ -30,6 +33,28 @@ public: > > } > > }; > > > > +/// Transforms a tweak into a code action that would apply > it if executed. > > +/// EXPECTS: T.prepare() was called and returned true. > > +CodeAction toCodeAction(const ClangdServer::TweakRef &T, > const URIForFile &File, > > + Range Selection) { > > + CodeAction CA; > > + CA.title = T.Title; > > + CA.kind = CodeAction::REFACTOR_KIND; > > + // This tweak may have an expensive second stage, we only > run it if the user > > + // actually chooses it in the UI. We reply with a command > that would run the > > + // corresponding tweak. > > + // FIXME: for some tweaks, computing the edits is cheap > and we could send them > > + // directly. > > + CA.command.emplace(); > > + CA.command->title = T.Title; > > + CA.command->command = Command::CLANGD_APPLY_TWEAK; > > + CA.command->tweakArgs.emplace(); > > + CA.command->tweakArgs->file = File; > > + CA.command->tweakArgs->tweakID = T.ID <http://T.ID>; > > + CA.command->tweakArgs->selection = Selection; > > + return CA; > > +}; > > + > > void > adjustSymbolKinds(llvm::MutableArrayRef<DocumentSymbol> Syms, > > SymbolKindBitset Kinds) { > > for (auto &S : Syms) { > > @@ -338,7 +363,9 @@ void ClangdLSPServer::onInitialize(const > > {"referencesProvider", true}, > > {"executeCommandProvider", > > llvm::json::Object{ > > - {"commands", > {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}}, > > + {"commands", > > + > {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND, > > + ExecuteCommandParams::CLANGD_APPLY_TWEAK}}, > > }}, > > }}}}); > > } > > @@ -400,7 +427,7 @@ void ClangdLSPServer::onFileEvent(const > > > > void ClangdLSPServer::onCommand(const ExecuteCommandParams > &Params, > > Callback<llvm::json::Value> > Reply) { > > - auto ApplyEdit = [&](WorkspaceEdit WE) { > > + auto ApplyEdit = [this](WorkspaceEdit WE) { > > ApplyWorkspaceEditParams Edit; > > Edit.edit = std::move(WE); > > // Ideally, we would wait for the response and if there > is no error, we > > @@ -420,6 +447,31 @@ void ClangdLSPServer::onCommand(const Ex > > > > Reply("Fix applied."); > > ApplyEdit(*Params.workspaceEdit); > > + } else if (Params.command == > ExecuteCommandParams::CLANGD_APPLY_TWEAK && > > + Params.tweakArgs) { > > + auto Code = > DraftMgr.getDraft(Params.tweakArgs->file.file()); > > + if (!Code) > > + return Reply(llvm::createStringError( > > + llvm::inconvertibleErrorCode(), > > + "trying to apply a code action for a non-added > file")); > > + > > + auto Action = [ApplyEdit](decltype(Reply) Reply, > URIForFile File, > > + std::string Code, > > + > llvm::Expected<tooling::Replacements> R) { > > + if (!R) > > + return Reply(R.takeError()); > > + > > + WorkspaceEdit WE; > > + WE.changes.emplace(); > > + (*WE.changes)[File.uri()] = replacementsToEdits(Code, *R); > > + > > + Reply("Fix applied."); > > + ApplyEdit(std::move(WE)); > > + }; > > + Server->applyTweak(Params.tweakArgs->file.file(), > > + Params.tweakArgs->selection, > Params.tweakArgs->tweakID, > > + Bind(Action, std::move(Reply), > Params.tweakArgs->file, > > + std::move(*Code))); > > } else { > > // We should not get here because ExecuteCommandParams > would not have > > // parsed in the first place and this handler should > not be called. But if > > @@ -601,28 +653,53 @@ static llvm::Optional<Command> asCommand > > > > void ClangdLSPServer::onCodeAction(const CodeActionParams > &Params, > > > Callback<llvm::json::Value> Reply) { > > - auto Code = DraftMgr.getDraft(Params.textDocument.uri.file()); > > + URIForFile File = Params.textDocument.uri; > > + auto Code = DraftMgr.getDraft(File.file()); > > if (!Code) > > return Reply(llvm::make_error<LSPError>( > > "onCodeAction called for non-added file", > ErrorCode::InvalidParams)); > > // We provide a code action for Fixes on the specified > diagnostics. > > - std::vector<CodeAction> Actions; > > + std::vector<CodeAction> FixIts; > > for (const Diagnostic &D : Params.context.diagnostics) { > > - for (auto &F : getFixes(Params.textDocument.uri.file(), > D)) { > > - Actions.push_back(toCodeAction(F, > Params.textDocument.uri)); > > - Actions.back().diagnostics = {D}; > > + for (auto &F : getFixes(File.file(), D)) { > > + FixIts.push_back(toCodeAction(F, > Params.textDocument.uri)); > > + FixIts.back().diagnostics = {D}; > > } > > } > > > > - if (SupportsCodeAction) > > - Reply(llvm::json::Array(Actions)); > > - else { > > - std::vector<Command> Commands; > > - for (const auto &Action : Actions) > > - if (auto Command = asCommand(Action)) > > - Commands.push_back(std::move(*Command)); > > - Reply(llvm::json::Array(Commands)); > > - } > > + // Now enumerate the semantic code actions. > > + auto ConsumeActions = > > + [this](decltype(Reply) Reply, URIForFile File, > std::string Code, > > + Range Selection, std::vector<CodeAction> FixIts, > > + > llvm::Expected<std::vector<ClangdServer::TweakRef>> Tweaks) { > > + if (!Tweaks) { > > + auto Err = Tweaks.takeError(); > > + if (Err.isA<CancelledError>()) > > + return Reply(std::move(Err)); // do no logging, > this is expected. > > + elog("error while getting semantic code actions: {0}", > > + std::move(Err)); > > + return Reply(llvm::json::Array(FixIts)); > > + } > > + > > + std::vector<CodeAction> Actions = std::move(FixIts); > > + Actions.reserve(Actions.size() + Tweaks->size()); > > + for (const auto &T : *Tweaks) > > + Actions.push_back(toCodeAction(T, File, Selection)); > > + > > + if (SupportsCodeAction) > > + return Reply(llvm::json::Array(Actions)); > > + std::vector<Command> Commands; > > + for (const auto &Action : Actions) { > > + if (auto Command = asCommand(Action)) > > + Commands.push_back(std::move(*Command)); > > + } > > + return Reply(llvm::json::Array(Commands)); > > + }; > > + > > + Server->enumerateTweaks(File.file(), Params.range, > > + Bind(ConsumeActions, std::move(Reply), > > + std::move(File), > std::move(*Code), Params.range, > > + std::move(FixIts))); > > } > > > > void ClangdLSPServer::onCompletion(const CompletionParams > &Params, > > > > Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=352494&r1=352493&r2=352494&view=diff > > > > ============================================================================== > > --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) > > +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jan > 29 06:17:36 2019 > > @@ -16,11 +16,13 @@ > > #include "XRefs.h" > > #include "index/FileIndex.h" > > #include "index/Merge.h" > > +#include "refactor/Tweak.h" > > #include "clang/Format/Format.h" > > #include "clang/Frontend/CompilerInstance.h" > > #include "clang/Frontend/CompilerInvocation.h" > > #include "clang/Lex/Preprocessor.h" > > #include "clang/Tooling/CompilationDatabase.h" > > +#include "clang/Tooling/Core/Replacement.h" > > #include > "clang/Tooling/Refactoring/RefactoringResultConsumer.h" > > #include "clang/Tooling/Refactoring/Rename/RenamingAction.h" > > #include "llvm/ADT/ArrayRef.h" > > @@ -28,10 +30,12 @@ > > #include "llvm/ADT/ScopeExit.h" > > #include "llvm/ADT/StringRef.h" > > #include "llvm/Support/Errc.h" > > +#include "llvm/Support/Error.h" > > #include "llvm/Support/FileSystem.h" > > #include "llvm/Support/Path.h" > > #include "llvm/Support/raw_ostream.h" > > #include <future> > > +#include <memory> > > #include <mutex> > > > > namespace clang { > > @@ -325,6 +329,56 @@ void ClangdServer::rename(PathRef File, > > "Rename", File, Bind(Action, File.str(), > NewName.str(), std::move(CB))); > > } > > > > +void ClangdServer::enumerateTweaks(PathRef File, Range Sel, > > + > Callback<std::vector<TweakRef>> CB) { > > + auto Action = [Sel](decltype(CB) CB, std::string File, > > + Expected<InputsAndAST> InpAST) { > > + if (!InpAST) > > + return CB(InpAST.takeError()); > > + > > + auto &AST = InpAST->AST; > > + auto CursorLoc = sourceLocationInMainFile( > > + AST.getASTContext().getSourceManager(), Sel.start); > > + if (!CursorLoc) > > + return CB(CursorLoc.takeError()); > > + Tweak::Selection Inputs = {InpAST->Inputs.Contents, > InpAST->AST, > > + *CursorLoc}; > > + > > + std::vector<TweakRef> Res; > > + for (auto &T : prepareTweaks(Inputs)) > > + Res.push_back({T->id(), T->title()}); > > + CB(std::move(Res)); > > + }; > > + > > + WorkScheduler.runWithAST("EnumerateTweaks", File, > > + Bind(Action, std::move(CB), > File.str())); > > +} > > + > > +void ClangdServer::applyTweak(PathRef File, Range Sel, > TweakID ID, > > + > Callback<tooling::Replacements> CB) { > > + auto Action = [ID, Sel](decltype(CB) CB, std::string File, > > + Expected<InputsAndAST> InpAST) { > > + if (!InpAST) > > + return CB(InpAST.takeError()); > > + > > + auto &AST = InpAST->AST; > > + auto CursorLoc = sourceLocationInMainFile( > > + AST.getASTContext().getSourceManager(), Sel.start); > > + if (!CursorLoc) > > + return CB(CursorLoc.takeError()); > > + Tweak::Selection Inputs = {InpAST->Inputs.Contents, > InpAST->AST, > > + *CursorLoc}; > > + > > + auto A = prepareTweak(ID, 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())); > > +} > > + > > void ClangdServer::dumpAST(PathRef File, > > > llvm::unique_function<void(std::string)> Callback) { > > auto Action = [](decltype(Callback) Callback, > > > > Modified: clang-tools-extra/trunk/clangd/ClangdServer.h > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=352494&r1=352493&r2=352494&view=diff > > > > ============================================================================== > > --- clang-tools-extra/trunk/clangd/ClangdServer.h (original) > > +++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Jan 29 > 06:17:36 2019 > > @@ -21,8 +21,10 @@ > > #include "index/Background.h" > > #include "index/FileIndex.h" > > #include "index/Index.h" > > +#include "refactor/Tweak.h" > > #include "clang/Tooling/CompilationDatabase.h" > > #include "clang/Tooling/Core/Replacement.h" > > +#include "llvm/ADT/FunctionExtras.h" > > #include "llvm/ADT/IntrusiveRefCntPtr.h" > > #include "llvm/ADT/Optional.h" > > #include "llvm/ADT/StringRef.h" > > @@ -211,6 +213,18 @@ public: > > void rename(PathRef File, Position Pos, llvm::StringRef > NewName, > > Callback<std::vector<tooling::Replacement>> CB); > > > > + struct TweakRef { > > + TweakID 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. > > + void enumerateTweaks(PathRef File, Range Sel, > > + Callback<std::vector<TweakRef>> CB); > > + > > + /// Apply the code tweak with a specified \p ID. > > + void applyTweak(PathRef File, Range Sel, TweakID ID, > > + Callback<tooling::Replacements> CB); > > + > > /// Only for testing purposes. > > /// Waits until all requests to worker thread are > finished and dumps AST for > > /// \p File. \p File must be in the list of added documents. > > > > Modified: clang-tools-extra/trunk/clangd/Protocol.cpp > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=352494&r1=352493&r2=352494&view=diff > > > > ============================================================================== > > --- clang-tools-extra/trunk/clangd/Protocol.cpp (original) > > +++ clang-tools-extra/trunk/clangd/Protocol.cpp Tue Jan 29 > 06:17:36 2019 > > @@ -421,6 +421,9 @@ bool fromJSON(const llvm::json::Value &P > > > > const llvm::StringLiteral > ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND = > > "clangd.applyFix"; > > +const llvm::StringLiteral > ExecuteCommandParams::CLANGD_APPLY_TWEAK = > > + "clangd.applyTweak"; > > + > > bool fromJSON(const llvm::json::Value &Params, > ExecuteCommandParams &R) { > > llvm::json::ObjectMapper O(Params); > > if (!O || !O.map("command", R.command)) > > @@ -431,6 +434,8 @@ bool fromJSON(const llvm::json::Value &P > > return Args && Args->size() == 1 && > > fromJSON(Args->front(), R.workspaceEdit); > > } > > + if (R.command == ExecuteCommandParams::CLANGD_APPLY_TWEAK) > > + return Args && Args->size() == 1 && > fromJSON(Args->front(), R.tweakArgs); > > return false; // Unrecognized command. > > } > > > > @@ -497,10 +502,13 @@ llvm::json::Value toJSON(const Command & > > auto Cmd = llvm::json::Object{{"title", C.title}, > {"command", C.command}}; > > if (C.workspaceEdit) > > Cmd["arguments"] = {*C.workspaceEdit}; > > + if (C.tweakArgs) > > + Cmd["arguments"] = {*C.tweakArgs}; > > return std::move(Cmd); > > } > > > > const llvm::StringLiteral CodeAction::QUICKFIX_KIND = > "quickfix"; > > +const llvm::StringLiteral CodeAction::REFACTOR_KIND = > "refactor"; > > > > llvm::json::Value toJSON(const CodeAction &CA) { > > auto CodeAction = llvm::json::Object{{"title", CA.title}}; > > @@ -544,6 +552,17 @@ llvm::json::Value toJSON(const Workspace > > return llvm::json::Object{{"changes", > std::move(FileChanges)}}; > > } > > > > +bool fromJSON(const llvm::json::Value &Params, TweakArgs &A) { > > + llvm::json::ObjectMapper O(Params); > > + return O && O.map("file", A.file) && O.map("selection", > A.selection) && > > + O.map("tweakID", A.tweakID); > > +} > > + > > +llvm::json::Value toJSON(const TweakArgs &A) { > > + return llvm::json::Object{ > > + {"tweakID", A.tweakID}, {"selection", A.selection}, > {"file", A.file}}; > > +} > > + > > llvm::json::Value toJSON(const ApplyWorkspaceEditParams > &Params) { > > return llvm::json::Object{{"edit", Params.edit}}; > > } > > > > Modified: clang-tools-extra/trunk/clangd/Protocol.h > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=352494&r1=352493&r2=352494&view=diff > > > > ============================================================================== > > --- clang-tools-extra/trunk/clangd/Protocol.h (original) > > +++ clang-tools-extra/trunk/clangd/Protocol.h Tue Jan 29 > 06:17:36 2019 > > @@ -631,6 +631,21 @@ struct WorkspaceEdit { > > bool fromJSON(const llvm::json::Value &, WorkspaceEdit &); > > llvm::json::Value toJSON(const WorkspaceEdit &WE); > > > > +/// Arguments for the 'applyTweak' command. The server sends > these commands as a > > +/// response to the textDocument/codeAction request. The > client can later send a > > +/// command back to the server if the user requests to > execute a particular code > > +/// tweak. > > +struct TweakArgs { > > + /// A file provided by the client on a > textDocument/codeAction request. > > + URIForFile file; > > + /// A selection provided by the client on a > textDocument/codeAction request. > > + Range selection; > > + /// ID of the tweak that should be executed. Corresponds > to Tweak::id(). > > + std::string tweakID; > > +}; > > +bool fromJSON(const llvm::json::Value &, TweakArgs &); > > +llvm::json::Value toJSON(const TweakArgs &A); > > + > > /// Exact commands are not specified in the protocol so we > define the > > /// ones supported by Clangd here. The protocol specifies > the command arguments > > /// to be "any[]" but to make this safer and more > manageable, each command we > > @@ -642,12 +657,15 @@ llvm::json::Value toJSON(const Workspace > > struct ExecuteCommandParams { > > // Command to apply fix-its. Uses WorkspaceEdit as argument. > > const static llvm::StringLiteral CLANGD_APPLY_FIX_COMMAND; > > + // Command to apply the code action. Uses TweakArgs as > argument. > > + const static llvm::StringLiteral CLANGD_APPLY_TWEAK; > > > > /// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND > > std::string command; > > > > // Arguments > > llvm::Optional<WorkspaceEdit> workspaceEdit; > > + llvm::Optional<TweakArgs> tweakArgs; > > }; > > bool fromJSON(const llvm::json::Value &, > ExecuteCommandParams &); > > > > @@ -669,6 +687,7 @@ struct CodeAction { > > /// Used to filter code actions. > > llvm::Optional<std::string> kind; > > const static llvm::StringLiteral QUICKFIX_KIND; > > + const static llvm::StringLiteral REFACTOR_KIND; > > > > /// The diagnostics that this code action resolves. > > llvm::Optional<std::vector<Diagnostic>> diagnostics; > > > > Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=352494&r1=352493&r2=352494&view=diff > > > > ============================================================================== > > --- clang-tools-extra/trunk/clangd/SourceCode.cpp (original) > > +++ clang-tools-extra/trunk/clangd/SourceCode.cpp Tue Jan 29 > 06:17:36 2019 > > @@ -141,6 +141,16 @@ Position sourceLocToPosition(const Sourc > > return P; > > } > > > > +llvm::Expected<SourceLocation> > sourceLocationInMainFile(const SourceManager &SM, > > + > Position P) { > > + llvm::StringRef Code = > SM.getBuffer(SM.getMainFileID())->getBuffer(); > > + auto Offset = > > + positionToOffset(Code, P, > /*AllowColumnBeyondLineLength=*/false); > > + if (!Offset) > > + return Offset.takeError(); > > + return > SM.getLocForStartOfFile(SM.getMainFileID()).getLocWithOffset(*Offset); > > +} > > + > > Range halfOpenToRange(const SourceManager &SM, > CharSourceRange R) { > > // Clang is 1-based, LSP uses 0-based indexes. > > Position Begin = sourceLocToPosition(SM, R.getBegin()); > > > > Modified: clang-tools-extra/trunk/clangd/SourceCode.h > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.h?rev=352494&r1=352493&r2=352494&view=diff > > > > ============================================================================== > > --- clang-tools-extra/trunk/clangd/SourceCode.h (original) > > +++ clang-tools-extra/trunk/clangd/SourceCode.h Tue Jan 29 > 06:17:36 2019 > > @@ -56,6 +56,11 @@ Position offsetToPosition(llvm::StringRe > > /// FIXME: This should return an error if the location is > invalid. > > Position sourceLocToPosition(const SourceManager &SM, > SourceLocation Loc); > > > > +/// Return the file location, corresponding to \p P. Note > that one should take > > +/// care to avoid comparing the result with expansion locations. > > +llvm::Expected<SourceLocation> > sourceLocationInMainFile(const SourceManager &SM, > > + > Position P); > > + > > // Converts a half-open clang source range to an LSP range. > > // Note that clang also uses closed source ranges, which > this can't handle! > > Range halfOpenToRange(const SourceManager &SM, > CharSourceRange R); > > > > Added: clang-tools-extra/trunk/clangd/refactor/Tweak.cpp > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Tweak.cpp?rev=352494&view=auto > > > > ============================================================================== > > --- clang-tools-extra/trunk/clangd/refactor/Tweak.cpp (added) > > +++ clang-tools-extra/trunk/clangd/refactor/Tweak.cpp Tue Jan > 29 06:17:36 2019 > > @@ -0,0 +1,74 @@ > > +//===--- Tweak.cpp > -----------------------------------------------*- C++-*-===// > > +// > > +// Part of the LLVM Project, under the Apache License v2.0 > with LLVM Exceptions. > > +// See https://llvm.org/LICENSE.txt for license information. > > +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception > > +// > > > > +//===----------------------------------------------------------------------===// > > +#include "Tweak.h" > > +#include "Logger.h" > > +#include "llvm/ADT/STLExtras.h" > > +#include "llvm/ADT/StringMap.h" > > +#include "llvm/Support/Error.h" > > +#include "llvm/Support/Registry.h" > > +#include <functional> > > +#include <memory> > > + > > +LLVM_INSTANTIATE_REGISTRY(llvm::Registry<clang::clangd::Tweak>); > > + > > +namespace clang { > > +namespace clangd { > > + > > +/// A handy typedef to save some typing. > > +typedef llvm::Registry<Tweak> TweakRegistry; > > + > > +namespace { > > +/// Asserts invariants on TweakRegistry. No-op with > assertion disabled. > > +void validateRegistry() { > > +#ifndef NDEBUG > > + llvm::StringSet<> Seen; > > + for (const auto &E : TweakRegistry::entries()) { > > + // REGISTER_TWEAK ensures E.getName() is equal to the > tweak class name. > > + // We check that id() matches it. > > + assert(E.instantiate()->id() == E.getName() && > > + "id should be equal to class name"); > > + assert(Seen.try_emplace(E.getName()).second && > "duplicate check id"); > > + } > > +#endif > > +} > > +} // namespace > > + > > +std::vector<std::unique_ptr<Tweak>> prepareTweaks(const > Tweak::Selection &S) { > > + validateRegistry(); > > + > > + std::vector<std::unique_ptr<Tweak>> Available; > > + for (const auto &E : TweakRegistry::entries()) { > > + std::unique_ptr<Tweak> T = E.instantiate(); > > + if (!T->prepare(S)) > > + continue; > > + Available.push_back(std::move(T)); > > + } > > + // Ensure deterministic order of the results. > > + llvm::sort(Available, > > + [](const std::unique_ptr<Tweak> &L, > > + const std::unique_ptr<Tweak> &R) { return > L->id() < R->id(); }); > > + return Available; > > +} > > + > > +llvm::Expected<std::unique_ptr<Tweak>> prepareTweak(TweakID ID, > > + const > Tweak::Selection &S) { > > + auto It = llvm::find_if( > > + TweakRegistry::entries(), > > + [ID](const TweakRegistry::entry &E) { return > E.getName() == ID; }); > > + if (It == TweakRegistry::end()) > > + return > llvm::createStringError(llvm::inconvertibleErrorCode(), > > + "id of the tweak is > invalid"); > > + std::unique_ptr<Tweak> T = It->instantiate(); > > + if (!T->prepare(S)) > > + return > llvm::createStringError(llvm::inconvertibleErrorCode(), > > + "failed to prepare() a > check"); > > + return T; > > +} > > + > > +} // namespace clangd > > +} // namespace clang > > > > Added: clang-tools-extra/trunk/clangd/refactor/Tweak.h > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Tweak.h?rev=352494&view=auto > > > > ============================================================================== > > --- clang-tools-extra/trunk/clangd/refactor/Tweak.h (added) > > +++ clang-tools-extra/trunk/clangd/refactor/Tweak.h Tue Jan > 29 06:17:36 2019 > > @@ -0,0 +1,98 @@ > > +//===--- Tweak.h > -------------------------------------------------*- C++-*-===// > > +// > > +// Part of the LLVM Project, under the Apache License v2.0 > with LLVM Exceptions. > > +// See https://llvm.org/LICENSE.txt for license information. > > +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception > > +// > > > > +//===----------------------------------------------------------------------===// > > +// Tweaks are small refactoring-like actions that run over > the AST and produce > > +// the set of edits as a result. They are local, i.e. they > should take the > > +// current editor context, e.g. the cursor position and > selection into account. > > +// The actions are executed in two stages: > > +// - Stage 1 should check whether the action is available > in a current > > +// context. It should be cheap and fast to compute as it > is executed for all > > +// available actions on every client request, which > happen quite frequently. > > +// - Stage 2 is performed after stage 1 and can be more > expensive to compute. > > +// It is performed when the user actually chooses the > action. > > > > +//===----------------------------------------------------------------------===// > > + > > +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_TWEAK_H > > +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_TWEAK_H > > + > > +#include "ClangdUnit.h" > > +#include "Protocol.h" > > +#include "clang/Tooling/Core/Replacement.h" > > +#include "llvm/ADT/Optional.h" > > +#include "llvm/ADT/StringRef.h" > > +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. > > +/// // implement other methods here. > > +/// }; > > +/// REGISTER_TWEAK(MyTweak); > > +class Tweak { > > +public: > > + /// Input to prepare and apply tweaks. > > + struct Selection { > > + /// The text of the active document. > > + llvm::StringRef Code; > > + /// Parsed AST of the active file. > > + ParsedAST &AST; > > + /// A location of the cursor in the editor. > > + SourceLocation Cursor; > > + // FIXME: add selection when there are checks relying on it. > > + // FIXME: provide a way to get sources and ASTs for > other files. > > + // FIXME: cache some commonly required information (e.g. > AST nodes under > > + // cursor) to avoid redundant AST visit in every > action. > > + }; > > + virtual ~Tweak() = default; > > + /// 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; > > + /// 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. > > + /// This function should be fast, if the action requires > non-trivial work it > > + /// should be moved into 'apply'. > > + /// Returns true iff the action is available and apply() > can be called on it. > > + virtual bool prepare(const Selection &Sel) = 0; > > + /// Run the second stage of the action that would produce > the actual changes. > > + /// EXPECTS: prepare() was called and returned true. > > + virtual Expected<tooling::Replacements> apply(const > Selection &Sel) = 0; > > + /// A one-line title of the action that should be shown to > the users in the > > + /// UI. > > + /// EXPECTS: prepare() was called and returned true. > > + virtual std::string title() const = 0; > > +}; > > + > > +// All tweaks must be registered in the .cpp file next to > their definition. > > +#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); > \ > > + } > > + > > +/// Calls prepare() on all tweaks, returning those that can > run on the > > +/// selection. > > +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, > > + const > Tweak::Selection &S); > > + > > +} // namespace clangd > > +} // namespace clang > > + > > +#endif > > > > Added: > clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt?rev=352494&view=auto > > > > ============================================================================== > > --- > clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt > (added) > > +++ > clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt > Tue Jan 29 06:17:36 2019 > > @@ -0,0 +1,13 @@ > > +include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../..) > > + > > +# A target containing all code tweaks (i.e. > mini-refactorings) provided by > > +# clangd. > > +# Built as an object library to make sure linker does not > remove global > > +# constructors that register individual tweaks in a global > registry. > > +# To enable these tweaks in exectubales or shared libraries, add > > +# $<TARGET_OBJECTS:obj.clangDaemonTweaks> to a list of > sources, see > > +# clangd/tool/CMakeLists.txt for an example. > > +add_clang_library(clangDaemonTweaks OBJECT > > + Dummy.cpp # FIXME: to avoid CMake errors due to empty > inputs, remove when a > > + # first tweak lands. > > + ) > > > > Added: clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp?rev=352494&view=auto > > > > ============================================================================== > > --- clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp > (added) > > +++ clang-tools-extra/trunk/clangd/refactor/tweaks/Dummy.cpp > Tue Jan 29 06:17:36 2019 > > @@ -0,0 +1,9 @@ > > +//===--- Dummy.cpp > -----------------------------------------------*- C++-*-===// > > +// > > +// The LLVM Compiler Infrastructure > > +// > > +// This file is distributed under the University of Illinois > Open Source > > +// License. See LICENSE.TXT for details. > > +// > > > > +//===----------------------------------------------------------------------===// > > +// Does nothing, only here to avoid cmake errors for empty > libraries. > > \ No newline at end of file > > > > Modified: clang-tools-extra/trunk/clangd/tool/CMakeLists.txt > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/CMakeLists.txt?rev=352494&r1=352493&r2=352494&view=diff > > > > ============================================================================== > > --- clang-tools-extra/trunk/clangd/tool/CMakeLists.txt (original) > > +++ clang-tools-extra/trunk/clangd/tool/CMakeLists.txt Tue > Jan 29 06:17:36 2019 > > @@ -3,6 +3,7 @@ include_directories(${CMAKE_CURRENT_BINA > > > > add_clang_tool(clangd > > ClangdMain.cpp > > + $<TARGET_OBJECTS:obj.clangDaemonTweaks> > > ) > > > > set(LLVM_LINK_COMPONENTS > > > > Modified: clang-tools-extra/trunk/test/clangd/fixits-command.test > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/fixits-command.test?rev=352494&r1=352493&r2=352494&view=diff > > > > ============================================================================== > > --- clang-tools-extra/trunk/test/clangd/fixits-command.test > (original) > > +++ clang-tools-extra/trunk/test/clangd/fixits-command.test > Tue Jan 29 06:17:36 2019 > > @@ -23,7 +23,7 @@ > > # CHECK-NEXT: "uri": "file://{{.*}}/foo.c" > > # CHECK-NEXT: } > > --- > > > > -{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": > {"line": 0, "character": 32}, "end": {"line": 0, "character": > 37}},"severity":2,"message":"Using the result of an assignment > as a condition without parentheses"}]}}} > > > > +{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":0,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": > {"line": 0, "character": 32}, "end": {"line": 0, "character": > 37}},"severity":2,"message":"Using the result of an assignment > as a condition without parentheses"}]}}} > > # CHECK: "id": 2, > > # CHECK-NEXT: "jsonrpc": "2.0", > > # CHECK-NEXT: "result": [ > > @@ -92,7 +92,7 @@ > > # CHECK-NEXT: } > > # CHECK-NEXT: ] > > --- > > > > -{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": > {"line": 0, "character": 32}, "end": {"line": 0, "character": > 37}},"severity":2,"message":"Using the result of an assignment > as a condition without parentheses"}]}}} > > > > +{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":0,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": > {"line": 0, "character": 32}, "end": {"line": 0, "character": > 37}},"severity":2,"message":"Using the result of an assignment > as a condition without parentheses"}]}}} > > # Make sure unused "code" and "source" fields ignored > gracefully > > # CHECK: "id": 3, > > # CHECK-NEXT: "jsonrpc": "2.0", > > > > Modified: > clang-tools-extra/trunk/test/clangd/initialize-params.test > > URL: > > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/initialize-params.test?rev=352494&r1=352493&r2=352494&view=diff > > > > ============================================================================== > > --- > clang-tools-extra/trunk/test/clangd/initialize-params.test > (original) > > +++ > clang-tools-extra/trunk/test/clangd/initialize-params.test Tue > Jan 29 06:17:36 2019 > > @@ -25,7 +25,8 @@ > > # CHECK-NEXT: "documentSymbolProvider": true, > > # CHECK-NEXT: "executeCommandProvider": { > > # CHECK-NEXT: "commands": [ > > -# CHECK-NEXT: "clangd.applyFix" > > +# CHECK-NEXT: "clangd.applyFix", > > +# CHECK-NEXT: "clangd.applyTweak" > > # CHECK-NEXT: ] > > # CHECK-NEXT: }, > > # CHECK-NEXT: "hoverProvider": true, > > > > > > _______________________________________________ > > cfe-commits mailing list > > cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > > > -- > Regards, > Ilya Biryukov > > > > -- > Regards, > Ilya Biryukov _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
