ioeric added a comment. >> - 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. > > The added IndexRecordAction and existing IndexAction use the same > functionality from libIndex to collect the indexing data, so I'm not sure > mocking the unit writer to unit test IndexRecordAction would add very much > value – writing the index data out is the new behavior. The existing tests > for IndexAction (under test/Index/Core) are already covering the correctness > of the majority of the collected indexing info and the tests coming in the > follow-up patch (seen in the original version of this patch) test it's still > correct after the write/read round trip.
Thanks for the clarification! I still think it's useful to decouple the IndexAction from the bit format file so that it could be reusable elsewhere. For example, I can see the index action be useful to clangd for building in-memory index. I also tried applying your original patch locally but couldn't get it to work mostly due to portability issues (e.g. `blocks` and `if (APPLE)` in make files). AFAIK, many folks compile clang with GCC and/or without APPLE, so it's important that you get the portability right from the very beginning. Thanks! Index-while-build is awesome! I'm looking forward to your patches! https://reviews.llvm.org/D39050 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
