[clang-tools-extra] r352494 - [clangd] Interfaces for writing code tweaks
Mikael Holmén via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 30 01:05:50 PST 2019
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;
> + 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 at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 38b66ffe455.clang.log
Type: text/x-log
Size: 6938 bytes
Desc: 38b66ffe455.clang.log
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190130/715be725/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 38b66ffe455.gcc.log
Type: text/x-log
Size: 8333 bytes
Desc: 38b66ffe455.gcc.log
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190130/715be725/attachment-0003.bin>
More information about the cfe-commits
mailing list