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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
