[PATCH] D51628: [clangd] Implement a Proof-of-Concept tool for symbol index exploration

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.


================
Comment at: clang-tools-extra/clangd/dexplorer/Dexplorer.cpp:39
+
+// FIXME(kbobyrev): Make this an actual REPL: probably use LLVM Command Line
+// library for parsing flags and arguments.
----------------
ilya-biryukov wrote:
> Maybe we could expose `CommandLineParser` from `llvm/lib/Support/CommandLine.cpp` as a public API and use it here?
> Not sure if there are any obstacles to doing so or how much work is it, though.
> E.g. `cl::opt` seem to rely on being registered in the global parser and I'm not sure if there's an easy way out of it.
Yes, that might be tricky. I have thought about a few options, e.g. consuming the first token from the input string as the command and dispatching arguments parsed via `CommandLineParser` manually (discarding those which are not accepted by provided command, etc). There are still few complications: help wouldn't be separate and easily accessible (unless hardcoded, which is suboptimal).

Another approach I could think of would be simplifying the interface of each command so that it would be easily parsed:

* `> fuzzy-find request.json`: this would require `FuzzyFindRequest` JSON (de)serialization implementation, but that would be useful for the benchmark tool, too
* `> lookup-id symbolid`
* `> load symbol.yaml`/`swap symbol.yaml`
* `> density-hist`
* `> tokens-distribution`
* `> dense-tokens`
* `> sparse-tokens`

And also a generic `> help` for the short reference of each command. Out of all these commands, only Fuzzy Find request would benefit a lot from a proper parsing of arguments, having JSON file representation might not be ideal, but it looks OK for the first iteration for me. Fuzzy Find request would presumably be one of the most used commands, though, but then again, we could iterate if that is not really convinient.


================
Comment at: clang-tools-extra/clangd/dexplorer/Dexplorer.cpp:147
+
+  // FIXME(kbobyrev): Wrap time measurements into something like
+  // measureTime(Function, Arguments...).
----------------
ilya-biryukov wrote:
> +1 to this FIXME.
> 
> Something like:
> ```
> template <class Func>
> auto reportTime(StringRef Name, Func F) -> decltype(F()) {
>   auto Result = F();
>   llvm::outs() << Name << " took " << ...
>   return Result;
> } 
> ```
> 
> The code calling this API would be quite readable:
> ```
> auto Index = reportTime("Build stage", []() { 
>   return buildStaticIndex(...);
> });
> ```
Ah, this looks really clean! I had few ideas of how to do that, but I couldn't converge to a reasonable solution which wouldn't look like abusing C++ to me :) Thanks!


https://reviews.llvm.org/D51628