Author: ibiryukov Date: Fri Feb 16 04:20:47 2018 New Revision: 325337 URL: http://llvm.org/viewvc/llvm-project?rev=325337&view=rev Log: [clangd] Assert path is absolute when assigning to URIForFile.
Summary: The assertion will point directly to misbehaving code, so that debugging related problems (like the one fixed by r325029) is easier. Reviewers: hokein, ioeric, sammccall Reviewed By: sammccall Subscribers: klimek, jkorous-apple, cfe-commits Differential Revision: https://reviews.llvm.org/D43246 Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp clang-tools-extra/trunk/clangd/Protocol.cpp clang-tools-extra/trunk/clangd/Protocol.h clang-tools-extra/trunk/clangd/XRefs.cpp Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=325337&r1=325336&r2=325337&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Feb 16 04:20:47 2018 @@ -87,8 +87,8 @@ std::vector<TextEdit> replacementsToEdit } // namespace void ClangdLSPServer::onInitialize(InitializeParams &Params) { - if (Params.rootUri && !Params.rootUri->file.empty()) - Server.setRootPath(Params.rootUri->file); + if (Params.rootUri && *Params.rootUri) + Server.setRootPath(Params.rootUri->file()); else if (Params.rootPath && !Params.rootPath->empty()) Server.setRootPath(*Params.rootPath); @@ -136,9 +136,9 @@ void ClangdLSPServer::onExit(ExitParams void ClangdLSPServer::onDocumentDidOpen(DidOpenTextDocumentParams &Params) { if (Params.metadata && !Params.metadata->extraFlags.empty()) - CDB.setExtraFlagsForFile(Params.textDocument.uri.file, + CDB.setExtraFlagsForFile(Params.textDocument.uri.file(), std::move(Params.metadata->extraFlags)); - Server.addDocument(Params.textDocument.uri.file, Params.textDocument.text); + Server.addDocument(Params.textDocument.uri.file(), Params.textDocument.text); } void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params) { @@ -146,7 +146,7 @@ void ClangdLSPServer::onDocumentDidChang return replyError(ErrorCode::InvalidParams, "can only apply one change at a time"); // We only support full syncing right now. - Server.addDocument(Params.textDocument.uri.file, + Server.addDocument(Params.textDocument.uri.file(), Params.contentChanges[0].text); } @@ -184,7 +184,7 @@ void ClangdLSPServer::onCommand(ExecuteC } void ClangdLSPServer::onRename(RenameParams &Params) { - Path File = Params.textDocument.uri.file; + Path File = Params.textDocument.uri.file(); llvm::Optional<std::string> Code = Server.getDocument(File); if (!Code) return replyError(ErrorCode::InvalidParams, @@ -206,12 +206,12 @@ void ClangdLSPServer::onRename(RenamePar } void ClangdLSPServer::onDocumentDidClose(DidCloseTextDocumentParams &Params) { - Server.removeDocument(Params.textDocument.uri.file); + Server.removeDocument(Params.textDocument.uri.file()); } void ClangdLSPServer::onDocumentOnTypeFormatting( DocumentOnTypeFormattingParams &Params) { - auto File = Params.textDocument.uri.file; + auto File = Params.textDocument.uri.file(); auto Code = Server.getDocument(File); if (!Code) return replyError(ErrorCode::InvalidParams, @@ -227,7 +227,7 @@ void ClangdLSPServer::onDocumentOnTypeFo void ClangdLSPServer::onDocumentRangeFormatting( DocumentRangeFormattingParams &Params) { - auto File = Params.textDocument.uri.file; + auto File = Params.textDocument.uri.file(); auto Code = Server.getDocument(File); if (!Code) return replyError(ErrorCode::InvalidParams, @@ -242,7 +242,7 @@ void ClangdLSPServer::onDocumentRangeFor } void ClangdLSPServer::onDocumentFormatting(DocumentFormattingParams &Params) { - auto File = Params.textDocument.uri.file; + auto File = Params.textDocument.uri.file(); auto Code = Server.getDocument(File); if (!Code) return replyError(ErrorCode::InvalidParams, @@ -259,14 +259,14 @@ void ClangdLSPServer::onDocumentFormatti void ClangdLSPServer::onCodeAction(CodeActionParams &Params) { // We provide a code action for each diagnostic at the requested location // which has FixIts available. - auto Code = Server.getDocument(Params.textDocument.uri.file); + auto Code = Server.getDocument(Params.textDocument.uri.file()); if (!Code) return replyError(ErrorCode::InvalidParams, "onCodeAction called for non-added file"); json::ary Commands; for (Diagnostic &D : Params.context.diagnostics) { - auto Edits = getFixIts(Params.textDocument.uri.file, D); + auto Edits = getFixIts(Params.textDocument.uri.file(), D); if (!Edits.empty()) { WorkspaceEdit WE; WE.changes = {{Params.textDocument.uri.uri(), std::move(Edits)}}; @@ -281,12 +281,12 @@ void ClangdLSPServer::onCodeAction(CodeA } void ClangdLSPServer::onCompletion(TextDocumentPositionParams &Params) { - Server.codeComplete(Params.textDocument.uri.file, Params.position, CCOpts, + Server.codeComplete(Params.textDocument.uri.file(), Params.position, CCOpts, [](Tagged<CompletionList> List) { reply(List.Value); }); } void ClangdLSPServer::onSignatureHelp(TextDocumentPositionParams &Params) { - Server.signatureHelp(Params.textDocument.uri.file, Params.position, + Server.signatureHelp(Params.textDocument.uri.file(), Params.position, [](llvm::Expected<Tagged<SignatureHelp>> SignatureHelp) { if (!SignatureHelp) return replyError( @@ -298,7 +298,7 @@ void ClangdLSPServer::onSignatureHelp(Te void ClangdLSPServer::onGoToDefinition(TextDocumentPositionParams &Params) { Server.findDefinitions( - Params.textDocument.uri.file, Params.position, + Params.textDocument.uri.file(), Params.position, [](llvm::Expected<Tagged<std::vector<Location>>> Items) { if (!Items) return replyError(ErrorCode::InvalidParams, @@ -308,13 +308,13 @@ void ClangdLSPServer::onGoToDefinition(T } void ClangdLSPServer::onSwitchSourceHeader(TextDocumentIdentifier &Params) { - llvm::Optional<Path> Result = Server.switchSourceHeader(Params.uri.file); + llvm::Optional<Path> Result = Server.switchSourceHeader(Params.uri.file()); reply(Result ? URI::createFile(*Result).toString() : ""); } void ClangdLSPServer::onDocumentHighlight(TextDocumentPositionParams &Params) { Server.findDocumentHighlights( - Params.textDocument.uri.file, Params.position, + Params.textDocument.uri.file(), Params.position, [](llvm::Expected<Tagged<std::vector<DocumentHighlight>>> Highlights) { if (!Highlights) return replyError(ErrorCode::InternalError, Modified: clang-tools-extra/trunk/clangd/Protocol.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=325337&r1=325336&r2=325337&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Protocol.cpp (original) +++ clang-tools-extra/trunk/clangd/Protocol.cpp Fri Feb 16 04:20:47 2018 @@ -12,8 +12,8 @@ //===----------------------------------------------------------------------===// #include "Protocol.h" -#include "URI.h" #include "Logger.h" +#include "URI.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/Format.h" @@ -24,6 +24,11 @@ namespace clang { namespace clangd { +URIForFile::URIForFile(std::string AbsPath) { + assert(llvm::sys::path::is_absolute(AbsPath) && "the path is relative"); + File = std::move(AbsPath); +} + bool fromJSON(const json::Expr &E, URIForFile &R) { if (auto S = E.asString()) { auto U = URI::parse(*S); @@ -40,15 +45,13 @@ bool fromJSON(const json::Expr &E, URIFo log(llvm::toString(Path.takeError())); return false; } - R.file = *Path; + R = URIForFile(*Path); return true; } return false; } -json::Expr toJSON(const URIForFile &U) { - return U.uri(); -} +json::Expr toJSON(const URIForFile &U) { return U.uri(); } llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const URIForFile &U) { return OS << U.uri(); Modified: clang-tools-extra/trunk/clangd/Protocol.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=325337&r1=325336&r2=325337&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Protocol.h (original) +++ clang-tools-extra/trunk/clangd/Protocol.h Fri Feb 16 04:20:47 2018 @@ -49,12 +49,17 @@ enum class ErrorCode { }; struct URIForFile { - std::string file; + URIForFile() = default; + explicit URIForFile(std::string AbsPath); - std::string uri() const { return URI::createFile(file).toString(); } + /// Retrieves absolute path to the file. + llvm::StringRef file() const { return File; } + + explicit operator bool() const { return !File.empty(); } + std::string uri() const { return URI::createFile(File).toString(); } friend bool operator==(const URIForFile &LHS, const URIForFile &RHS) { - return LHS.file == RHS.file; + return LHS.File == RHS.File; } friend bool operator!=(const URIForFile &LHS, const URIForFile &RHS) { @@ -62,8 +67,11 @@ struct URIForFile { } friend bool operator<(const URIForFile &LHS, const URIForFile &RHS) { - return LHS.file < RHS.file; + return LHS.File < RHS.File; } + +private: + std::string File; }; /// Serialize/deserialize \p URIForFile to/from a string URI. Modified: clang-tools-extra/trunk/clangd/XRefs.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=325337&r1=325336&r2=325337&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/XRefs.cpp (original) +++ clang-tools-extra/trunk/clangd/XRefs.cpp Fri Feb 16 04:20:47 2018 @@ -149,7 +149,7 @@ getDeclarationLocation(ParsedAST &AST, c } } - L.uri.file = FilePath.str(); + L.uri = URIForFile(FilePath.str()); L.range = R; return L; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits