[PATCH] D84499: [clangd] Add more logs and attach tracers to remote index server routines

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:96
+    } Counter(Request->DebugString());
+    return std::forward<IndexCall>(Call)(
+        *Index, ClangdRequest, [&](const StreamType &Item) {
----------------
kbobyrev wrote:
> sammccall wrote:
> > I don't think you need forward here... just take the param by const reference?
> How do I do that? The problem is that some functions return `bool` and one `void`, so I can't really assign the result to variable and then return it.
Currently you have 
```
template <typename IndexCall>(IndexCall&& Call) {
  forward<IndexCall>(Call);
}
```
but I think this would work fine here:
```
template <typename IndexCall>(const IndexCall& Call) {
  IndexCall(Call);
}
```
(nothing particularly wrong with this use of forward, but "more template goop" feels particularly expensive here.


================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:49
+    llvm::cl::desc("Pretty-print JSON output"),
+    llvm::cl::init(true),
+};
----------------
kbobyrev wrote:
> sammccall wrote:
> > kadircet wrote:
> > > this sounds like a debug feature, do we really want it to be true by default?
> > This option is not worth having at all, IMO.
> Why not? It's relatively hard to read trace files otherwise. Do you think they should be pretty-printed by default?
FWIW, I've never had to read them except when initially developing/debugging the trace feature.
(The most I have to do is fix up a broken end-of-file, and events are one-per-line in non-pretty anyway...)

I think either always-on or always-off is better than having a flag.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84499/new/

https://reviews.llvm.org/D84499