kbobyrev added inline comments.
================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:73
private:
+ template <typename RequestT, typename ReplyT, typename ClangdRequestT,
+ typename IndexCall, typename StreamType, typename CallbackT>
----------------
sammccall wrote:
> you don't need both RequestT and ClangdRequest as template params:
> - the things you're currently doing are all available through
> protobuf::Message base class
> - but why not move the request fromProtobuf call in here so you only need to
> pass a single parameter?
> but why not move the request fromProtobuf call in here so you only need to
> pass a single parameter?
Yeah this is the idea; I have D84525 for extending `fromProtobuf` so that it
can be moved here.
================
Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:75
+ typename IndexCall, typename StreamType, typename CallbackT>
+ typename std::result_of<IndexCall(clangd::SymbolIndex &,
+ const ClangdRequestT &, CallbackT)>::type
----------------
sammccall wrote:
> can't the return type just be `auto`?
> Then I think you don't need CallbackT as a template param.
Ah, I thought LLVM is still C++ 11, not 14. Good to know, thanks!
================
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) {
----------------
sammccall wrote:
> 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.
Ah, you meant passing the callback by const reference. I see, thanks for the
suggestion!
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
