This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Moved caching of compile commands to ClangdServer
ClosedPublic

Authored by ilya-biryukov on Jan 23 2018, 9:23 AM.

Event Timeline

ilya-biryukov created this revision.Jan 23 2018, 9:23 AM
sammccall accepted this revision.Jan 25 2018, 4:04 AM

Thanks for this cleanup!

The way we deal with ResourceDir still doesn't feel ideal, but that's no worse here and everything else is better!

clangd/ClangdServer.cpp
199

nit: the comment and whitespace seems unneccesary, this is exactly what invalidate() is documented to do.
(A comment saying *why* might be useful, but we don't really know why here)

272–273

nit: remove blank line?

clangd/ClangdServer.h
340

nit: can we hide this in the cpp file or move to GCD.h to reduce clutter?

340

Can the comment give some motivation here? It's clear enough from the name what it does, but why?

341

sorry for naive question - this doesn't need to be threadsafe, because clangdserver itself isn't, right?
I never really looked at the threading model of clangdserver - it looks like most stuff is called externally on one thread, and any async logic avoids accessing most of the members?

This revision is now accepted and ready to land.Jan 25 2018, 4:04 AM
ilya-biryukov marked 5 inline comments as done.
  • Addressed review comments
This revision was automatically updated to reflect the committed changes.