hokein added inline comments.
================
Comment at: clangd/ClangdLSPServer.cpp:258
+ if (!Items)
+ return replyError(ErrorCode::InvalidParams,
+ llvm::toString(Items.takeError()));
----------------
`InvalidParams` doesn't match all cases where internal errors occur. Maybe use
`ErrorCode::InternalError`?
================
Comment at: clangd/FindSymbols.cpp:145
+ if (!Uri) {
+ log(llvm::formatv(
+ "Workspace symbol: Could not parse URI '{0}' for symbol '{1}'.",
----------------
I think we may want to report the error to client instead of just logging them.
================
Comment at: clangd/SourceCode.cpp:143
+/// From "a::b::c", return {"a::b::", "c"}. Scope is empty if there's no
+/// qualifier.
+std::pair<llvm::StringRef, llvm::StringRef>
----------------
nit: this comment is duplicated with the one in header.
================
Comment at: clangd/SourceCode.h:59
+/// Turn a pair of offsets into a Location.
+llvm::Optional<Location>
+offsetRangeToLocation(const DraftStore &DS, SourceManager &SourceMgr,
----------------
We should use `llvm::Expected`?
The function needs a comment documenting its behavior, especially on the
unsaved file content.
================
Comment at: clangd/SourceCode.h:61
+offsetRangeToLocation(const DraftStore &DS, SourceManager &SourceMgr,
+ StringRef File, size_t OffsetStart, size_t OffsetEnd);
+
----------------
nit: name `FilePath` or `FileName`, `File` seems to be a bit confusing, does it
mean `FileContent` or `FileName`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44882
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
