[PATCH] D34512: Add preliminary Cross Translation Unit support library
Gábor Horváth via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 21 01:10:51 PDT 2017
xazax.hun added a comment.
In https://reviews.llvm.org/D34512#877032, @dcoughlin wrote:
> I'm OK with this going into the repo a is (although it is light on tests!), as long as we have an agreement that you'll be OK with iteration on both the interface and the implementation to handle real-world projects.
I agree that it could have more tests, but while the main features are already tested, more tests are also coming with the first usage of the library (the Static Analyzer). We are also willing to add new tests as the interface/library evolves.
> More specifically, for this to work well on large/complicated projects we'll need to:
>
> - Move conflict resolution from index generation where it is now to query time so that clients can pick the best implementation of a function when multiple are found. This is important for projects that have multiple functions with the same name or that build the same file in multiple ways.
I agree. Right now the way we use this internally, the collisions are handled after index generation by an external tool (CodeChecker).
> - Change function lookup from needing to traverse over the entire AST.
> - Move away from a linear, flat-file index to something that can handle larger projects more efficiently.
>
> For this last point, I suspect it will be good to adopt whatever Clangd ends up using to support indexing. (There has been some discussion of this relatively recently on the cfe-dev list.)
The index right now is a minimal implementation to make this functionality work. This part was never the bottleneck according to our measurements so far. It would be great to reuse the index format from ClangD in case it supports to generate lightweight indexes that only contain the data we need for analysis. (We only need an index of function definitions for the analyzer and nothing else. I suspect other clients also would not need other information.)
I think it would be a waste of effort to change the lookup strategy or move the collision handling mechanisms to index generation until we do not know how the index will work. But I also think it wouldn't be good to block this until ClanD indexing reaching a mature state.
All in all, we are willing to reuse as much of ClangD indexing as we can in the future, but I think it is also more aligned with the LLVM Developer Policy to have something working committed and do the development in the repo rather than working separately on a fork.
https://reviews.llvm.org/D34512
More information about the cfe-commits
mailing list