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

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D39050#900830, @arphaman wrote:

> I think this patch should be split into a number of smaller patches to help the review process.
>
> Things like `tools/IndexStore`, `DirectoryWatcher` and other components that are not directly needed right now should definitely be in their own patches.
>  It would be nice to find some way to split the implementation into multiple patches as well.


+1.

This is a lot of work (but great work!) for one patch. Smaller/incremental patches help reviewers understand and (hopefully) capture potential improvement of the design. I would really appreciate it if you could further split the patch.

Some comments/ideas:

- The lack of tests is a bit concerning.
- I think the implementation of the index output logic (e.g. `IndexUnitWriter` and bit format file) can be abstracted away (and split into separate patches) so that you can unit-test the action with a custom/mock unit writer; this would also make the action reusable with (potentially) other storage formats.
- I would suggest that you start with a patch that implement the index action and just enough components so that you could test the action.

Thanks!


https://reviews.llvm.org/D39050