[PATCH] D43550: [clangd] Not collect include headers for dynamic index for now.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added reviewers: ilya-biryukov, sammccall, hokein.
Herald added subscribers: cfe-commits, jkorous-apple, klimek.

The new behaviors introduced by this patch:
o When include collection is enabled, we always set IncludeHeader field in Symbol
even if it's the same as FileURI in decl.
o Disable include collection in FileIndex which is currently only used to build
dynamic index. We should revisit when we actually want to use FileIndex to global
index.
o Code-completion only uses IncludeHeader to insert headers but not FileURI in
CanonicalDeclaration. This ensures that inserted headers are always canonicalized.
Note that include insertion can still be triggered for symbols that are already
included if they are merged from dynamic index and static index, but we would
only use includes that are already canonicalized (e.g. from static index).

Reason for change:
Collecting header includes in dynamic index enables inserting includes for headers
that are not indexed but opened in the editor. Comparing to inserting includes for
symbols in global/static index, this is nice-to-have but would probably require
non-trivial amount of work to get right. For example:
o Currently it's not easy to fully support CanonicalIncludes in dynamic index, given the way
we run dynamic index.
o It's also harder to reason about the correctness of include canonicalization for dynamic index
(i.e. symbols in the current file/TU) than static index where symbols are collected
offline and sanity check is possible before shipping to production.
o We have less control/flexibility over symbol info in the dynamic index
(e.g. URIs, path normalization), which could be used to help make decision when inserting includes.

As header collection (especially canonicalization) is relatively new, and enabling
it for dynamic index would immediately affect current users with only dynamic
index support, I propose we disable it for dynamic index for now to avoid
compromising other hot features like code completion and only support it for
static index where include insertion would likely to bring more value.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43550

Files:
  clangd/CodeComplete.cpp
  clangd/index/FileIndex.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43550.135210.patch
Type: text/x-patch
Size: 6862 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180221/2b6557c3/attachment.bin>