[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