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.
On Wed, Jan 30, 2019 at 12:12 PM Ilya Biryukov <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> > 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; >> > + 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 >> > 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
