kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/index/remote/Client.cpp:61
Callback(*Response);
+ ++Received;
}
----------------
shouldn't we increment this before the `continue` above ? (or rename this to
`successful` rather than `received` ?)
================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:42
+llvm::cl::opt<std::string> TracerFile(
+ "tracer-file",
----------------
i think `TraceFile` is more appropriate here.
================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:48
+ "pretty",
+ llvm::cl::desc("Pretty-print JSON output"),
+ llvm::cl::init(true),
----------------
is it only for pretty printing trace files? do we expect to have any other
components this will interact in future? (if not maybe state that in the
comments)
================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:49
+ llvm::cl::desc("Pretty-print JSON output"),
+ llvm::cl::init(true),
+};
----------------
this sounds like a debug feature, do we really want it to be true by default?
================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:122
}
- Index->lookup(Req, [&](const clangd::Symbol &Sym) {
- auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
- if (!SerializedSymbol)
- return;
- LookupReply NextMessage;
- *NextMessage.mutable_stream_result() = *SerializedSymbol;
- Reply->Write(NextMessage);
- });
+ std::function<void(clang::clangd::SymbolIndex &,
+ const clang::clangd::LookupRequest &,
----------------
all of this trickery is really nice. but i am not sure if it improves
readability at all, from the perspective of call sites the amount of code is
almost the same. moreover it was some trivial lambda before and instead it is
lots of template parameters now.
maybe just have templated function to replace the lambda body instead? e.g.
```
template <typename StreamType, typenameResultType>
void IndexCallback(Marshaller &M, StreamType &Item) {
trace::Span Tracer....;
auto SerializedSymbol = M.toProtobuf(Item);
// log the events and much more ...
}
```
then call it in the callbacks. WDYT?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84499/new/
https://reviews.llvm.org/D84499
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
