[PATCH] D39050: Add index-while-building support to Clang

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

Hi! I got a bit further in my experiment in integrating this in Clangd. I put some comments (in the first more complete revision). But since the scope of this patch changed, if you feel like we should take the discussions elsewhere, please let me know! Thanks!



================
Comment at: include/indexstore/IndexStoreCXX.h:84
+
+  std::pair<unsigned, unsigned> getLineCol() {
+    unsigned line, col;
----------------
>From what I understand, this returns the beginning of the occurrence. It would be useful to also have the end of the occurrence. From what I tested in Xcode, when you do "Find Selected Symbol in Workspace", it highlights the symbol name in yellow in the list of matches, so it mush use that LineCol then highlight the matching name. This is works in many situations but others occurrences won't have the name of the symbol. For example:
"MyClass o1, o2;"
If I use "Find Selected Symbol in Workspace" on MyClass constructor, if won't be able to highlight o1 and o2
Do you think it would be possible to add that (EndLineCol)? If not, how would one go about extending libindexstore in order to add additional information per occurrence? It is not obvious to me so far.
We also need other things, for example in the case of definitions, we need the full range of the definition so that users can "peek" at definitions in a pop-up. Without storing the end of the definition in the index, we would need to reparse the file.



================
Comment at: include/indexstore/IndexStoreCXX.h:374
+
+  enum class DependencyKind {
+    Unit,
----------------
As part of this dependency tracking mechanism, I haven't found that it could provide information about about the files including a specific header. So given a header (or source file in odd cases), I do not see a way to get all the files that would need to be reindexed if that header changed. Is that something you achieve outside the index? Or perhaps this is something I missed in the code.


================
Comment at: include/indexstore/IndexStoreCXX.h:377
+    Record,
+    File,
+  };
----------------
Could there be a bit of explanation about what's a File dependency versus record and unit? All units and records are file dependencies, right? Are there any files that are neither records or units?


https://reviews.llvm.org/D39050