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

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 11 03:43:27 PDT 2018


ilya-biryukov added a comment.

Dexplorer is a long name. How about shortening it to `dexp`?



================
Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:56
+
+template <class Function, class TimeUnit = std::chrono::milliseconds>
+void reportTime(StringRef Name, Function F) {
----------------
Why do we want `TimeUnit`?
It adds complexity, if we want to make large values more readable we have other alternatives:
- printing adaptive units, e.g. "when it's larger than 5000ms, start printing seconds", "when it's larger than 5minutes, start printing minutes"
- provide separators (`500000000ms`  is pretty much unreadable, `500 000 000ms` is much better)



================
Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:125
+// * print out tokens with least dense posting lists
+void dispatchRequest(const std::unique_ptr<SymbolIndex> &Index,
+                     StringRef Request) {
----------------
This function does too many things:
- parses the input.
- dispatches on different kinds of requests.
- handles user input errors.
- actually runs the commands.

This turns out somewhat unreadable at the end, we might want to separate those into multiple functions. Will need a bit of time to think on how to actually do it best. Will come back after I have concrete suggestions.


================
Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:139
+                      "symbol name.\n";
+      helpRequest();
+      return;
----------------
This produces a ton of output.
Could we simply produce a message to run 'help' to get a list of supported commands instead?

It could be really annoying to see large help messages on every typo.


https://reviews.llvm.org/D51628





More information about the cfe-commits mailing list