This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Simplify Dex query tree logic and fix missing-posting-list bug
ClosedPublic

Authored by sammccall on Oct 2 2018, 12:49 PM.

Details

Summary

The bug being fixed: when a posting list doesn't exist in the index, it
was previously just dropped from the query rather than being treated as
empty. Now that we have the FALSE iterator, we can use it instead.

The query tree logic previously had a bunch of special cases to detect whether
subtrees are empty. Now we just naively build the whole tree, and rely
on the query optimizations to drop the trivial parts.

Finally, there was a bug in trigram generation: the empty query would
generate a single trigram "$$$" instead of no trigrams.
This had no effect (there was no posting list, so the other bug
cancelled it out). But we now have to fix this bug too.

Event Timeline

sammccall created this revision.Oct 2 2018, 12:49 PM
sammccall updated this revision to Diff 168290.Oct 4 2018, 7:28 AM

Rebase, and enable test that is now waiting on this patch.

Overall LGTM, just a quick question to make sure I get what's going on.

clangd/index/dex/Dex.cpp
173

Why do we still need to push all() with a trivial boost for Scope.empty()? Is this a legacy way to specify "all scopes"?

ilya-biryukov accepted this revision.Oct 4 2018, 8:53 AM

Putting a stamp, assuming is answer is "yes, it's a legacy way to specify all scopes"

This revision is now accepted and ready to land.Oct 4 2018, 8:53 AM
sammccall added inline comments.Oct 4 2018, 10:19 AM
clangd/index/dex/Dex.cpp
173

Yeah. This is specified in FuzzyFindRequest, with a FIXME to remove as we have explicit AnyScope (but we still have code relying on it for now).

This revision was automatically updated to reflect the committed changes.