[PATCH] D51676: [clangd] Use TopN instead of std::priority_queue

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 5 13:00:50 PDT 2018


sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks for cleaning this up!

I believe this will result in the results from MemIndex being returned in best -> worst order, rather than worst -> best.
The contract says callers shouldn't rely on the order so it should still be NFC overall.
It's likely to make your index explorer more useful though :-)



================
Comment at: clang-tools-extra/clangd/index/MemIndex.cpp:42
     if (auto Score = Filter.match(Sym->Name)) {
-      Top.emplace(-*Score * quality(*Sym), Sym);
-      if (Top.size() > Req.MaxCandidateCount) {
+      // Top.push(...) returns true if the capacity is reached and the heap had
+      // to pop() and item before inserting a new one.
----------------
this looks more like generic documentation for Top::push.
I'd suggest instead adding a line comment when it's true:
`More = true; // We discarded something.`


https://reviews.llvm.org/D51676





More information about the cfe-commits mailing list