[PATCH] D34512: Add preliminary Cross Translation Unit support library

Devin Coughlin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 29 21:24:53 PDT 2017


dcoughlin added a comment.

> The importing alters the ASTContext, so the only way to choose between the definitions would be via a callback that is triggered before the import is done. What do you think?

I think that could work. Another possibility would be to have a two step process. The first step would return a representation of possible definitions to import; then the client would chose which one to important and call another API to perform the actual import.

In either case, the important scenario I think we should support is choosing at a call site to a C function the most likely definition of the called function, based on number and type of parameters, from multiple possible definitions in other translation units. If the API is rich enough to support this then I think that is a good indication it will be useful for other scenarios as well.

> The reason we introduced the diagnostic is that we assumed that the client can not recover from a configuration error and will end up reporting the problem to the user.

For the static analyzer client, at least, one possible recovery is performing the existing invalidation that we do when calling a function defined in another translation unit. (I'm not really sure this a good idea; I just wanted to point out that the decision probably belongs with the client). I think it is reasonable to report an error as a diagnostic -- but I think this should be up to the client and I don't think it should show up in the index file itself. In an ideal world the user wouldn't even know that file existed. Further, for large projects/incremental rebuilds a text file is unlikely to be a suitable representation for the index. In the long term I doubt the file will be end-user editable/readable.


https://reviews.llvm.org/D34512





More information about the cfe-commits mailing list