Add preliminary Cross Translation Unit support library

[xazax.hun]

Summary

This patch introduces a class that can help to build tools that require cross translation unit facilities.
This class allows function definitions to be loaded from external AST files based on an index.
In order to use this functionality an index is required. USRs are used as names to look up the functions.
This class also does caching to avoid redundant loading of AST files.

Right now only function defnitions can be loaded using this API, because this is what the Static Analyzer requires.
In to future this could be extended to classes, types etc.

More tests will come together with the first user once it is accepted: https://reviews.llvm.org/D30691

[xazax.hun]

• Add a tool to dump USRs for function definitions. It can be used to create index files.

[klimek]

General direction looks good.

I think in addition to integration tests through the static analyzer, we should still have unit tests.

include/clang/Tooling/CrossTranslationUnit.h

47 ↗

(On Diff #103571)

I'd just spell out Cross & s/CTUDIR/IndexDir/ perhaps? (if CTUDir is to contain the indices)
I think as this is the major API entry point, a comment that explains the meaning of the arguments and how to use them would be nice.

[xazax.hun]

• Added test for the USR dumping tool.
• Added unit test for the CrossTranslationUnit class
• Added documentation for the API entry point
• Renamed some functions and variables

[xazax.hun]

Unfortunately, in order to make the Unit Test pass, I also had to modify the AST Importer. Are you ok with having that change as part of this patch (Aleksei might be a suitable person to review that part) or should I make a separate differential for that?

[klimek]

Adding folks interested in the indexing discussion.

include/clang/Tooling/CrossTranslationUnit.h

53–58 ↗

(On Diff #103707)

In the future we'll want to create an index interface around this (which will probably serve also what the refactoring integration would be based on), instead of piping files and directories into all classes.

Perhaps we can start this by already pulling out a class ProjectIndex or somesuch, with methods like loadASTDefining(...)?

[xazax.hun]

include/clang/Tooling/CrossTranslationUnit.h

53–58 ↗

(On Diff #103707)

While I do agree to have an interface for that would be really good, but maybe it would be better to first review and accept this patch and after that design the interface in a follow-up patch (so https://reviews.llvm.org/D30691 is not blocked). What do you think?

[xazax.hun]

• Removed an unrelated change
• Made the unit test more strict

[klimek]

include/clang/Tooling/CrossTranslationUnit.h

53–58 ↗

(On Diff #103707)

I'm generally fine with that, and mainly looped in other folks to see whether anybody else has concerns.

[xazax.hun]

• Updated to compile with trunk
• Minor style fixes
• Proper diagnostic when the index format is wrong

[xazax.hun]

It looks like Richard approved libTooling as a dependency for clang on the mailing list (http://lists.llvm.org/pipermail/cfe-dev/2017-July/054536.html).
If it is ok to have this code in libTooling (for now), I think we could start/continue the review of this patch.

[klimek]

In D34512#800490, @xazax.hun wrote:

It looks like Richard approved libTooling as a dependency for clang on the mailing list (http://lists.llvm.org/pipermail/cfe-dev/2017-July/054536.html).
If it is ok to have this code in libTooling (for now), I think we could start/continue the review of this patch.

I read that somewhat differently? It seems like Richard basically proposes adding a new library for things that control how we run clang in a multi-TU scenario. I'd call it libIndex, but that already exists :)

[xazax.hun]

In D34512#800499, @klimek wrote:

In D34512#800490, @xazax.hun wrote:

It looks like Richard approved libTooling as a dependency for clang on the mailing list (http://lists.llvm.org/pipermail/cfe-dev/2017-July/054536.html).
If it is ok to have this code in libTooling (for now), I think we could start/continue the review of this patch.

I read that somewhat differently? It seems like Richard basically proposes adding a new library for things that control how we run clang in a multi-TU scenario. I'd call it libIndex, but that already exists :)

No, but since Richard said he did not see any justifications to include this in libTooling, I had the impression this is not his final word, and in case you see it justified, this remains the suggested direction.
But in this case I will rewrite this patch to create a new library. Are you still interested in reviewing it? Do you have any other name in mind? What about libCrossTU?

[xazax.hun]

• Move CrossTU functionality into its own library

[klimek]

+Richard as top-level code owner for new libs.

For bikeshedding the name: I'd have liked libIndex, but that's already taken. CrossTU doesn't seem too bad to me, too, though.

[arphaman]

unittests/CrossTU/CMakeLists.txt

10 ↗

(On Diff #105409)

CrossTUTests?

[xazax.hun]

In D34512#800618, @klimek wrote:

+Richard as top-level code owner for new libs.

For bikeshedding the name: I'd have liked libIndex, but that's already taken. CrossTU doesn't seem too bad to me, too, though.

Some brainstorming for the bikeshedding: libProjet, libProjectIndex, libImport
The reason I am not sure about the Index in the name because this code does not contain (yet) an interface to create/handle indexes.
The import might be a good once, since this library is basically wrapping the ASTImporter to import code from external sources, but it might be misleading, since ASTImporter is in the AST module.

[whisperity]

In D34512#800502, @xazax.hun wrote:

In D34512#800499, @klimek wrote:

In D34512#800490, @xazax.hun wrote:

It looks like Richard approved libTooling as a dependency for clang on the mailing list (http://lists.llvm.org/pipermail/cfe-dev/2017-July/054536.html).
If it is ok to have this code in libTooling (for now), I think we could start/continue the review of this patch.

I read that somewhat differently? It seems like Richard basically proposes adding a new library for things that control how we run clang in a multi-TU scenario. I'd call it libIndex, but that already exists :)

No, but since Richard said he did not see any justifications to include this in libTooling, I had the impression this is not his final word, and in case you see it justified, this remains the suggested direction.
But in this case I will rewrite this patch to create a new library. Are you still interested in reviewing it? Do you have any other name in mind? What about libCrossTU?

In D34512#800625, @xazax.hun wrote:

In D34512#800618, @klimek wrote:

+Richard as top-level code owner for new libs.

For bikeshedding the name: I'd have liked libIndex, but that's already taken. CrossTU doesn't seem too bad to me, too, though.

Some brainstorming for the bikeshedding: libProjet, libProjectIndex, libImport
The reason I am not sure about the Index in the name because this code does not contain (yet) an interface to create/handle indexes.
The import might be a good once, since this library is basically wrapping the ASTImporter to import code from external sources, but it might be misleading, since ASTImporter is in the AST module.

Cross-TU is now used (in the lifeline of these features introduced by the team) in multiple contexts: CTU analysis, and now CTU lookup.
Maybe the name of this lib should reflect on the fact that the lib is used to support cross-TU definition lookup.

Hmm... libCrossTULocator?

[xazax.hun]

• Fix a copy and paste error and removed an unintended change.

[klimek]

In D34512#800626, @whisperity wrote:

In D34512#800502, @xazax.hun wrote:

In D34512#800499, @klimek wrote:

In D34512#800490, @xazax.hun wrote:

It looks like Richard approved libTooling as a dependency for clang on the mailing list (http://lists.llvm.org/pipermail/cfe-dev/2017-July/054536.html).
If it is ok to have this code in libTooling (for now), I think we could start/continue the review of this patch.

I read that somewhat differently? It seems like Richard basically proposes adding a new library for things that control how we run clang in a multi-TU scenario. I'd call it libIndex, but that already exists :)

No, but since Richard said he did not see any justifications to include this in libTooling, I had the impression this is not his final word, and in case you see it justified, this remains the suggested direction.
But in this case I will rewrite this patch to create a new library. Are you still interested in reviewing it? Do you have any other name in mind? What about libCrossTU?

In D34512#800625, @xazax.hun wrote:

In D34512#800618, @klimek wrote:

+Richard as top-level code owner for new libs.

For bikeshedding the name: I'd have liked libIndex, but that's already taken. CrossTU doesn't seem too bad to me, too, though.

Some brainstorming for the bikeshedding: libProjet, libProjectIndex, libImport
The reason I am not sure about the Index in the name because this code does not contain (yet) an interface to create/handle indexes.
The import might be a good once, since this library is basically wrapping the ASTImporter to import code from external sources, but it might be misleading, since ASTImporter is in the AST module.

Cross-TU is now used (in the lifeline of these features introduced by the team) in multiple contexts: CTU analysis, and now CTU lookup.
Maybe the name of this lib should reflect on the fact that the lib is used to support cross-TU definition lookup.

Hmm... libCrossTULocator?

Well, a better name for locator is index :)

[xazax.hun]

gentle ping

[klimek]

Specifically, ping Richard for new top-level lib in clang.

[xazax.hun]

In D34512#803724, @klimek wrote:

Specifically, ping Richard for new top-level lib in clang.

Richard proposed pulling this out into a separate library in the first place. Do we need his approval for the name? Or we want him to consider if this patch justifies the existence of a separate library?

[klimek]

Yep, I want Richard's approval for the name. I think he already expressed a pro-pulling-out stance.

[xazax.hun]

In D34512#806505, @klimek wrote:

Yep, I want Richard's approval for the name. I think he already expressed a pro-pulling-out stance.

Great! Ping for the name approval.

[rsmith]

Organizationally, this seems fine. Carry on :)

include/clang/Basic/DiagnosticFrontendKinds.td

229–231 ↗

(On Diff #105412)

Please add a new DiagnosticCrossTUKinds.td file for the new subdirectory.

include/clang/CrossTU/CrossTranslationUnit.h

13–14 ↗

(On Diff #105412)

This should be updated to match the new directory.

30 ↗

(On Diff #105412)

This doesn't follow either of our normal conventions for namespace names within Clang and LLVM (CamelCase and snake_case, with the latter being more common). Maybe cross_tu?

[xazax.hun]

• Addressed review comments.

[xazax.hun]

In D34512#829712, @rsmith wrote:

Organizationally, this seems fine. Carry on :)

Great news! Thank you!

[krasimir]

include/clang/CrossTU/CrossTUDiagnostic.h

16 ↗

(On Diff #109552)

LLVM Style uses no indent for namespaces. Reformat with clang-format.

include/clang/CrossTU/CrossTranslationUnit.h

70 ↗

(On Diff #109552)

Maybe rename to FunctionASTUnitMap for consistency with the previous line?

[xazax.hun]

• Address further review comments.

[whisperity]

Apart from those in the in-line comments, I have a question: how safe is this library to Release builds? I know this is only a submodule dependency for the "real deal" in D30691, but I have seen some asserts that "imported function should already have a body" and such.

Will the static analyzer handle these errors gracefully and fall back to "function is unknown, let's throw my presumptions out of the window" or will it bail away? I'm explicitly thinking of the assert-lacking Release build.

include/clang/Basic/AllDiagnostics.h

20 ↗

(On Diff #109559)

Import file order?

lib/CrossTU/CrossTranslationUnit.cpp

45 ↗

(On Diff #109559)

"visits (...) and looks up" or "visit (...) and look up"

tools/CMakeLists.txt

25 ↗

(On Diff #109559)

The other "blocks" in this file look somewhat in alphabetic order, perhaps this should be added a few lines above?

[whisperity]

include/clang/CrossTU/CrossTranslationUnit.h

42 ↗

(On Diff #109559)

Does the name of this class make sense? If I say

CrossTranslationUnit *ctuptr = new CrossTranslationUnit(...);

did I construct a new CrossTranslationUnit? What even is a CrossTranslationUnit? Other class names, such as ASTImporter, DiagOpts, and FrontendAction, etc. somehow feel better at transmitting reflecting upon their meaning in their name.

lib/CrossTU/CrossTranslationUnit.cpp

46 ↗

(On Diff #109559)

If we are using USR for lookup, then why does this look up "based on mangled name"?

[xazax.hun]

In D34512#836831, @whisperity wrote:

Apart from those in the in-line comments, I have a question: how safe is this library to Release builds? I know this is only a submodule dependency for the "real deal" in D30691, but I have seen some asserts that "imported function should already have a body" and such.

Will the static analyzer handle these errors gracefully and fall back to "function is unknown, let's throw my presumptions out of the window" or will it bail away? I'm explicitly thinking of the assert-lacking Release build.

The basic idea is that, if a function already have a body in the current translation unit and you still want to import it from somewhere else, it might be a programmer error, so we do not handle this gracefully.

include/clang/CrossTU/CrossTranslationUnit.h

42 ↗

(On Diff #109559)

What would you think about CrossTranslationUnitContext?

The functionality of this class is have all the relevant data required for doing Cross TU lookups and also provide the interface for that.

[whisperity]

include/clang/CrossTU/CrossTranslationUnit.h

42 ↗

(On Diff #109559)

WFM. I'm not sure whose authority is the class names, but I like it a lot better.

[xazax.hun]

• Address review comments

[dkrupp]

The creation of this library (libCrossTU) is approved for importing function definitions. @zaks.anna, @NoQ , @klimek could you please help us reviewing the code itself?

Then, when this is approved, we could progress with the review of the dependent "Support for naive cross translational unit analysis" (https://reviews.llvm.org/D30691) feature. The two patch is now in sync.
Thanks a lot in advance.

[dcoughlin]

Except for some drive-by nits, this is a high-level review.

I have some high-level questions about the design of the library:

1. How do you intend to handle the case when there are multiple function definitions with the same USR? Whose responsibility it is to deal with this: the client (for example, the static analyzer) or the index/database (libCrossTU)? This seems to me to be a fundamental part of the design for this feature that is missing. If you expect the client to handle this scenario (for example, to pick a definition) I would expect the library API to return more than a single potential result for a query. If you expect the the library to pick a definition then at a minimum you should document how this will be done. If the library will treat this as an error then you should communicate this to the client so it can attempt to recover from it.

2. Whose responsibility is it to handle errors that the library runs into? Right now several errors appear to reported to the end user as diagnostics. It seems to me that the decision of how (or whether) to report these errors to the end user should be up to a particular client. Are these errors even actionable on the part of end users? My sense it that with the envisioned workflow users would not know/care about the format of the database file.

3. Where should the code that understands the database file format live? Right now the logic for the parsing of the index format is in libCrossTU and the emission of the index is in the command-line tool. I think it would be easier to maintain if there were a single place that understand the file format. This way consumers and emitters of the index could be easily updated when the format/representation changes. Additionally, I think it is important to document this format.

include/clang/Basic/DiagnosticCrossTUKinds.td

13 ↗

(On Diff #110559)

Is this a user-facing diagnostic? Do we expect users to know what 'CrossTU' means? Or what 'USR' means?

include/clang/CrossTU/CrossTranslationUnit.h

60 ↗

(On Diff #110559)

Can this document what happens when the function declaration cannot be found? And what happens when multiple function declarations are found?

lib/CrossTU/CrossTranslationUnit.cpp

48 ↗

(On Diff #110559)

Should this assert that DC is not null? What is the reason to accept null here?

81 ↗

(On Diff #110559)

What is the rationale behind emitting this as a diagnostic? In general for a library I would expect that errors would be communicated to the client, which then would have responsibility for handling them, reporting them to the user, or aborting.

92 ↗

(On Diff #110559)

It looks like this is parsing. Is the file format documented anywhere? Also, it would be good to keep the parsing and generation code in the same place so that it is easy to figure out what to update when the file format changes.

tools/clang-func-mapping/ClangFnMapGen.cpp

86 ↗

(On Diff #110559)

It seems like the functionality that writes an entry and the functionality that reads an entry should ideally be in the same place. This way when the format is updated it is obvious what other places need to be updated. Can the code that writes a mapping be moved to libCrossTU and can we have this tool link against it?

[xazax.hun]

In D34512#854729, @dcoughlin wrote:

Except for some drive-by nits, this is a high-level review.

I have some high-level questions about the design of the library:

1. How do you intend to handle the case when there are multiple function definitions with the same USR? Whose responsibility it is to deal with this: the client (for example, the static analyzer) or the index/database (libCrossTU)? This seems to me to be a fundamental part of the design for this feature that is missing. If you expect the client to handle this scenario (for example, to pick a definition) I would expect the library API to return more than a single potential result for a query. If you expect the the library to pick a definition then at a minimum you should document how this will be done. If the library will treat this as an error then you should communicate this to the client so it can attempt to recover from it.

Right now we rely on the script that is using the analyzer such as scan-build-py or codechecker to remove the duplicate USRs from the index before starting the analysis. Having an ErrorOr return type so the user can handle the case when multiple definitions or none of them is found could be useful. It is not feasible to return multiple definitions, however. 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?

2. Whose responsibility is it to handle errors that the library runs into? Right now several errors appear to reported to the end user as diagnostics. It seems to me that the decision of how (or whether) to report these errors to the end user should be up to a particular client. Are these errors even actionable on the part of end users? My sense it that with the envisioned workflow users would not know/care about the format of the database file.

The intention was that to report to the user that the index format is invalid, since it is a configuration issue that she might be able to handle. But we could also return an error code and leave the reporting to the client. 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.

3. Where should the code that understands the database file format live? Right now the logic for the parsing of the index format is in libCrossTU and the emission of the index is in the command-line tool. I think it would be easier to maintain if there were a single place that understand the file format. This way consumers and emitters of the index could be easily updated when the format/representation changes. Additionally, I think it is important to document this format.

This is a very good point! I will update the library.

include/clang/Basic/DiagnosticCrossTUKinds.td

13 ↗

(On Diff #110559)

It is. Do you have any alternative suggestion? What about error parsing index file without mentioning any of the terms you mentioned?

include/clang/CrossTU/CrossTranslationUnit.h

60 ↗

(On Diff #110559)

Sure. Good point.

lib/CrossTU/CrossTranslationUnit.cpp

48 ↗

(On Diff #110559)

Indeed, an assert should be better.

81 ↗

(On Diff #110559)

Since this is likely to be a configuration error we were thinking that only the user can handle it. But I could transform this to an error code if you would prefer that solution.

92 ↗

(On Diff #110559)

Good point!

tools/clang-func-mapping/ClangFnMapGen.cpp

86 ↗

(On Diff #110559)

Sure, good point!

[xazax.hun]

• Updates according to review comments.

[dcoughlin]

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.

[xazax.hun]

In D34512#856184, @dcoughlin wrote:

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.

Note that the lookup is already based on USR which is similar to mangled names in a sense that it contains information about the function parameters. So the only way to get multiple candidates from the lookup is having multiple function definitions with the same signature. It is only possible to disambiguate with knowledge about the linking graph. I think that information is better to be stored in the index in the future and do the disambiguation within the library instead of making the client do that.

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.

I agree that we do not consider this format to be final for the index. In the future, it would be great to include linking information as well and also have utilities to merge partial indexes. This is a first version to make the machinery work for the most common cases which are already pretty usable for the open source projects we tested.

[xazax.hun]

• Added unit test to ensure the produced index format can be parsed.
• Added further diagnostics.

[benlangmuir]

In D34512#856301, @xazax.hun wrote:

In D34512#856184, @dcoughlin wrote:

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.

Note that the lookup is already based on USR which is similar to mangled names in a sense that it contains information about the function parameters. So the only way to get multiple candidates from the lookup is having multiple function definitions with the same signature.

I just want to clarify that C function USRs do not contain type information, although C++ USRs do.

[xazax.hun]

In D34512#856821, @benlangmuir wrote:

In D34512#856301, @xazax.hun wrote:

In D34512#856184, @dcoughlin wrote:

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.

Note that the lookup is already based on USR which is similar to mangled names in a sense that it contains information about the function parameters. So the only way to get multiple candidates from the lookup is having multiple function definitions with the same signature.

I just want to clarify that C function USRs do not contain type information, although C++ USRs do.

Just double checked and indeed: https://github.com/llvm-mirror/clang/blob/master/lib/Index/USRGeneration.cpp#L236
Thank you for the clarification, it looks like I did not notice this detail. In this case, it might make sense to let the client disambiguate.

[danielmarjamaki]

small nits

include/clang/CrossTU/CrossTranslationUnit.h

58 ↗

(On Diff #113206)

I suggest that "can" is removed.

62 ↗

(On Diff #113206)

there is no \return or \returns here.

62 ↗

(On Diff #113206)

maybe: each line consists of an USR and a filepath separated by a space

68 ↗

(On Diff #113206)

Maybe /can be/is/ unless you envision that tools that require cross translation unit capability might use some other classes.

92 ↗

(On Diff #113206)

you should split out some of this information to a \return or \returns

lib/CrossTU/CrossTranslationUnit.cpp

60 ↗

(On Diff #113206)

redundant assignment

79 ↗

(On Diff #113206)

I suggest that LineNo is 1 on the first line.

81 ↗

(On Diff #113206)

Pos can be const

82 ↗

(On Diff #113206)

LineRef can be const

84 ↗

(On Diff #113206)

FunctionName and FileName can be moved here and it is possible to make these variables const.

85 ↗

(On Diff #113206)

I would like to see some FunctionName validation. For instance "123" should not be a valid function name.

89 ↗

(On Diff #113206)

Stupid question .. how will this work if the path is longer than 256 bytes?

102 ↗

(On Diff #113206)

how about std::ostringstream , imho that is cleaner

121 ↗

(On Diff #113206)

/funtion/function/

125 ↗

(On Diff #113206)

LookupFnName could be const right?

148 ↗

(On Diff #113206)

I believe LookupFnName can be const

189 ↗

(On Diff #113206)

It is possible to make ASTFileName const

223 ↗

(On Diff #113206)

sorry I am probably missing something here.. you first assert !FD->hasBody() and here you assert FD->hasBody(). Is it changed in this function somewhere?

tools/clang-func-mapping/ClangFnMapGen.cpp

78 ↗

(On Diff #113206)

I believe that realpath() is a posix function

unittests/CrossTU/CrossTranslationUnitTest.cpp

40 ↗

(On Diff #113206)

should this be assert(FD && FD->getName() == "f")

[xazax.hun]

• Make the API capable of using custom lookup strategies for CTU
• Fix review comments

lib/CrossTU/CrossTranslationUnit.cpp

82 ↗

(On Diff #113206)

StringRef is an immutable reference to a string, I think it is not idiomatic in LLVM codebase to make it const.

85 ↗

(On Diff #113206)

This is not a real function name but an USR. I updated the name of the variable to reflect that this name is only used for lookup.

89 ↗

(On Diff #113206)

If the path is shorter than 256 bytes it will be stored on stack, otherwise SmallString will allocate on the heap.

125 ↗

(On Diff #113206)

In LLVM we usually do not mark StringRefs as consts, they represent a const reference to a string.

223 ↗

(On Diff #113206)

Yes, after the importer imports the new declaration with a body we should be able to find the body through the original declaration. The importer modifies the AST and the supporting data structures.

[dcoughlin]

Thanks Gabor! Some additional comments in line.

include/clang/CrossTU/CrossTranslationUnit.h

118 ↗

(On Diff #113740)

Is this comment correct? Does it return a map?

128 ↗

(On Diff #113740)

I found this comment confusing. Is 'Unit' the separate ASTUnit or it the current one?

134 ↗

(On Diff #113740)

If this is public API, it deserves a comment. If not, can we make it private?

lib/CrossTU/CrossTranslationUnit.cpp

35 ↗

(On Diff #113740)

Is this FIXME still relevant? What will need to be transitioned to llvm::Error for it to be removed? Can you make the comment a bit more specific so that future maintainers will know when/how to address the FIXME?

194 ↗

(On Diff #113740)

What is this cast for? Is it to suppress an analyzer dead store false positive? I thought we got all those!

199 ↗

(On Diff #113740)

If I understand correctly, there is no way to call loadExternalAST() without the potential for a diagnostic showing up to the user. Can this diagnostic emission be moved to the client?

249 ↗

(On Diff #113740)

Does this method need to take the unit? Can it just get the ASTContext from 'FD'? If not, can we add an assertion that FD has the required relationship to Unit? (And document it in the header doc.)

tools/clang-func-mapping/ClangFnMapGen.cpp

56 ↗

(On Diff #113740)

Is getLookupName() here used?

72 ↗

(On Diff #113740)

Should this instead reuse the logic for generating a lookup name from CrossTranslationUnitContext::getLookupName()? That way if we change how the lookup name is generated we only have to do it in one place.

[xazax.hun]

• Address review comments.

lib/CrossTU/CrossTranslationUnit.cpp

35 ↗

(On Diff #113740)

Unfortunately, it is. IndexError needs to implement convertToErrorCode because it is pure virtual in the base class, and that requires the existence of this class. So this can only be removed, once convertToErrorCode is pruned globally from the codebase. I think all of such classes are marked with similar FIXMEs.

194 ↗

(On Diff #113740)

The point was to avoid an assertion fail where an error was not checked. The reason, there is no really good way to propagate errors right now. But with this diagnostic emission moved to the client this is no longer required.

[xazax.hun]

Any further reviews?
What are the criteria to accept this patch? Who or how many people should accept this?

[dcoughlin]

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.

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.
• 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.)

[xazax.hun]

In 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.

[xazax.hun]

• Unittests now creates temporary files at the correct location
• Temporary files are also cleaned up when the process is terminated
• Absolute paths are handled correctly by the library

[dcoughlin]

But I also think it wouldn't be good to block this until ClanD indexing reaching a mature state.

I agree 100%.

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.

This sounds good to me. I just wanted to make sure that you were onboard with changing this index to use the common indexing infrastructure in the future. (Assuming it makes sense to do so, of course).

[r.stahl]

In D34512#877643, @xazax.hun wrote:

• Unittests now creates temporary files at the correct location
• Temporary files are also cleaned up when the process is terminated
• Absolute paths are handled correctly by the library

I think there is an issue with this change.

First, the function map generation writes absolute paths to the map file. Now when the parseCrossTUIndex function parses it, it will no longer prepend the paths with the CTUDir and therefore expect the precompiled AST files at the exact path of the source file. This seems like a bad requirement, because the user would have to overwrite his source files.

If I understand your intention correctly, a fix would be to replace the is_absolute check with a check for CTUDir == "" in the parseCrossTUIndex function.

[xazax.hun]

In D34512#890537, @r.stahl wrote:

In D34512#877643, @xazax.hun wrote:

• Unittests now creates temporary files at the correct location
• Temporary files are also cleaned up when the process is terminated
• Absolute paths are handled correctly by the library

I think there is an issue with this change.

First, the function map generation writes absolute paths to the map file. Now when the parseCrossTUIndex function parses it, it will no longer prepend the paths with the CTUDir and therefore expect the precompiled AST files at the exact path of the source file. This seems like a bad requirement, because the user would have to overwrite his source files.

If I understand your intention correctly, a fix would be to replace the is_absolute check with a check for CTUDir == "" in the parseCrossTUIndex function.

Good catch, that is right!

cfe/trunk/include/clang/Basic/AllDiagnostics.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_ALLDIAGNOSTICS_H
 #define LLVM_CLANG_BASIC_ALLDIAGNOSTICS_H
 
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/CommentDiagnostic.h"
 #include "clang/Analysis/AnalysisDiagnostic.h"
+#include "clang/CrossTU/CrossTUDiagnostic.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "clang/Serialization/SerializationDiagnostic.h"
 
 namespace clang {

cfe/trunk/include/clang/Basic/CMakeLists.txt

 macro(clang_diag_gen component)
   clang_tablegen(Diagnostic${component}Kinds.inc
     -gen-clang-diags-defs -clang-component=${component}
     SOURCE Diagnostic.td
     TARGET ClangDiagnostic${component})
 endmacro(clang_diag_gen)
 
 clang_diag_gen(Analysis)
 clang_diag_gen(AST)
 clang_diag_gen(Comment)
 clang_diag_gen(Common)
+clang_diag_gen(CrossTU)
 clang_diag_gen(Driver)
 clang_diag_gen(Frontend)
 clang_diag_gen(Lex)
 clang_diag_gen(Parse)
 clang_diag_gen(Sema)
 clang_diag_gen(Serialization)
 clang_tablegen(DiagnosticGroups.inc -gen-clang-diag-groups
   SOURCE Diagnostic.td

cfe/trunk/include/clang/Basic/Diagnostic.td

 }
 class DefaultRemark { Severity DefaultSeverity = SEV_Remark; }
 
 // Definitions for Diagnostics.
 include "DiagnosticASTKinds.td"
 include "DiagnosticAnalysisKinds.td"
 include "DiagnosticCommentKinds.td"
 include "DiagnosticCommonKinds.td"
+include "DiagnosticCrossTUKinds.td"
 include "DiagnosticDriverKinds.td"
 include "DiagnosticFrontendKinds.td"
 include "DiagnosticLexKinds.td"
 include "DiagnosticParseKinds.td"
 include "DiagnosticSemaKinds.td"
 include "DiagnosticSerializationKinds.td"
 

cfe/trunk/include/clang/Basic/DiagnosticCrossTUKinds.td

+//==--- DiagnosticCrossTUKinds.td - Cross Translation Unit diagnostics ----===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+let Component = "CrossTU" in {
+
+def err_fnmap_parsing : Error<
+  "error parsing index file: '%0' line: %1 'UniqueID filename' format "
+  "expected">;
+
+def err_multiple_def_index : Error<
+  "multiple definitions are found for the same key in index ">;
+}

cfe/trunk/include/clang/Basic/DiagnosticIDs.h

enum {
       DIAG_SIZE_COMMON        =  300,
       DIAG_SIZE_DRIVER        =  200,
       DIAG_SIZE_FRONTEND      =  100,
       DIAG_SIZE_SERIALIZATION =  120,
       DIAG_SIZE_LEX           =  400,
       DIAG_SIZE_PARSE         =  500,
       DIAG_SIZE_AST           =  110,
       DIAG_SIZE_COMMENT       =  100,
+      DIAG_SIZE_CROSSTU       =  100,
       DIAG_SIZE_SEMA          = 3500,
       DIAG_SIZE_ANALYSIS      =  100
     };
     // Start position for diagnostics.
     enum {
       DIAG_START_COMMON        =                          0,
       DIAG_START_DRIVER        = DIAG_START_COMMON        + DIAG_SIZE_COMMON,
       DIAG_START_FRONTEND      = DIAG_START_DRIVER        + DIAG_SIZE_DRIVER,
       DIAG_START_SERIALIZATION = DIAG_START_FRONTEND      + DIAG_SIZE_FRONTEND,
       DIAG_START_LEX           = DIAG_START_SERIALIZATION + DIAG_SIZE_SERIALIZATION,
       DIAG_START_PARSE         = DIAG_START_LEX           + DIAG_SIZE_LEX,
       DIAG_START_AST           = DIAG_START_PARSE         + DIAG_SIZE_PARSE,
       DIAG_START_COMMENT       = DIAG_START_AST           + DIAG_SIZE_AST,
-      DIAG_START_SEMA          = DIAG_START_COMMENT       + DIAG_SIZE_COMMENT,
+      DIAG_START_CROSSTU       = DIAG_START_COMMENT       + DIAG_SIZE_CROSSTU,
+      DIAG_START_SEMA          = DIAG_START_CROSSTU       + DIAG_SIZE_COMMENT,
       DIAG_START_ANALYSIS      = DIAG_START_SEMA          + DIAG_SIZE_SEMA,
       DIAG_UPPER_LIMIT         = DIAG_START_ANALYSIS      + DIAG_SIZE_ANALYSIS
     };
 
     class CustomDiagInfo;
 
     /// \brief All of the diagnostics that can be emitted by the frontend.
     typedef unsigned kind;

cfe/trunk/include/clang/CrossTU/CrossTUDiagnostic.h

+//===--- CrossTUDiagnostic.h - Diagnostics for Cross TU ---------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_CROSSTU_CROSSTUDIAGNOSTIC_H
+#define LLVM_CLANG_CROSSTU_CROSSTUDIAGNOSTIC_H
+
+#include "clang/Basic/Diagnostic.h"
+
+namespace clang {
+namespace diag {
+enum {
+#define DIAG(ENUM, FLAGS, DEFAULT_MAPPING, DESC, GROUP, SFINAE, NOWERROR,      \
+             SHOWINSYSHEADER, CATEGORY)                                        \
+  ENUM,
+#define CROSSTUSTART
+#include "clang/Basic/DiagnosticCrossTUKinds.inc"
+#undef DIAG
+  NUM_BUILTIN_CROSSTU_DIAGNOSTICS
+};
+} // end namespace diag
+} // end namespace clang
+
+#endif // LLVM_CLANG_FRONTEND_FRONTENDDIAGNOSTIC_H

cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h

+//===--- CrossTranslationUnit.h - -------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file provides an interface to load binary AST dumps on demand. This
+//  feature can be utilized for tools that require cross translation unit
+//  support.
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_CLANG_CROSSTU_CROSSTRANSLATIONUNIT_H
+#define LLVM_CLANG_CROSSTU_CROSSTRANSLATIONUNIT_H
+
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+class CompilerInstance;
+class ASTContext;
+class ASTImporter;
+class ASTUnit;
+class DeclContext;
+class FunctionDecl;
+class NamedDecl;
+class TranslationUnitDecl;
+
+namespace cross_tu {
+
+enum class index_error_code {
+  unspecified = 1,
+  missing_index_file,
+  invalid_index_format,
+  multiple_definitions,
+  missing_definition,
+  failed_import,
+  failed_to_get_external_ast,
+  failed_to_generate_usr
+};
+
+class IndexError : public llvm::ErrorInfo<IndexError> {
+public:
+  static char ID;
+  IndexError(index_error_code C) : Code(C), LineNo(0) {}
+  IndexError(index_error_code C, std::string FileName, int LineNo = 0)
+      : Code(C), FileName(std::move(FileName)), LineNo(LineNo) {}
+  void log(raw_ostream &OS) const override;
+  std::error_code convertToErrorCode() const override;
+  index_error_code getCode() const { return Code; }
+  int getLineNum() const { return LineNo; }
+  std::string getFileName() const { return FileName; }
+
+private:
+  index_error_code Code;
+  std::string FileName;
+  int LineNo;
+};
+
+/// \brief This function parses an index file that determines which
+///        translation unit contains which definition.
+///
+/// The index file format is the following:
+/// each line consists of an USR and a filepath separated by a space.
+///
+/// \return Returns a map where the USR is the key and the filepath is the value
+///         or an error.
+llvm::Expected<llvm::StringMap<std::string>>
+parseCrossTUIndex(StringRef IndexPath, StringRef CrossTUDir);
+
+std::string createCrossTUIndexString(const llvm::StringMap<std::string> &Index);
+
+/// \brief This class is used for tools that requires cross translation
+///        unit capability.
+///
+/// This class can load function definitions from external AST files.
+/// The loaded definition will be merged back to the original AST using the
+/// AST Importer.
+/// In order to use this class, an index file is required that describes
+/// the locations of the AST files for each function definition.
+///
+/// Note that this class also implements caching.
+class CrossTranslationUnitContext {
+public:
+  CrossTranslationUnitContext(CompilerInstance &CI);
+  ~CrossTranslationUnitContext();
+
+  /// \brief This function loads a function definition from an external AST
+  ///        file and merge it into the original AST.
+  ///
+  /// This method should only be used on functions that have no definitions in
+  /// the current translation unit. A function definition with the same
+  /// declaration will be looked up in the index file which should be in the
+  /// \p CrossTUDir directory, called \p IndexName. In case the declaration is
+  /// found in the index the corresponding AST file will be loaded and the
+  /// definition of the function will be merged into the original AST using
+  /// the AST Importer.
+  ///
+  /// \return The declaration with the definition will be returned.
+  /// If no suitable definition is found in the index file or multiple
+  /// definitions found error will be returned.
+  ///
+  /// Note that the AST files should also be in the \p CrossTUDir.
+  llvm::Expected<const FunctionDecl *>
+  getCrossTUDefinition(const FunctionDecl *FD, StringRef CrossTUDir,
+                       StringRef IndexName);
+
+  /// \brief This function loads a function definition from an external AST
+  ///        file.
+  ///
+  /// A function definition with the same declaration will be looked up in the
+  /// index file which should be in the \p CrossTUDir directory, called
+  /// \p IndexName. In case the declaration is found in the index the
+  /// corresponding AST file will be loaded.
+  ///
+  /// \return Returns an ASTUnit that contains the definition of the looked up
+  /// function.
+  ///
+  /// Note that the AST files should also be in the \p CrossTUDir.
+  llvm::Expected<ASTUnit *> loadExternalAST(StringRef LookupName,
+                                            StringRef CrossTUDir,
+                                            StringRef IndexName);
+
+  /// \brief This function merges a definition from a separate AST Unit into
+  ///        the current one which was created by the compiler instance that
+  ///        was passed to the constructor.
+  ///
+  /// \return Returns the resulting definition or an error.
+  llvm::Expected<const FunctionDecl *> importDefinition(const FunctionDecl *FD);
+
+  /// \brief Get a name to identify a function.
+  static std::string getLookupName(const NamedDecl *ND);
+
+  /// \brief Emit diagnostics for the user for potential configuration errors.
+  void emitCrossTUDiagnostics(const IndexError &IE);
+
+private:
+  ASTImporter &getOrCreateASTImporter(ASTContext &From);
+  const FunctionDecl *findFunctionInDeclContext(const DeclContext *DC,
+                                                StringRef LookupFnName);
+
+  llvm::StringMap<std::unique_ptr<clang::ASTUnit>> FileASTUnitMap;
+  llvm::StringMap<clang::ASTUnit *> FunctionASTUnitMap;
+  llvm::StringMap<std::string> FunctionFileMap;
+  llvm::DenseMap<TranslationUnitDecl *, std::unique_ptr<ASTImporter>>
+      ASTUnitImporterMap;
+  CompilerInstance &CI;
+  ASTContext &Context;
+};
+
+} // namespace cross_tu
+} // namespace clang
+
+#endif // LLVM_CLANG_CROSSTU_CROSSTRANSLATIONUNIT_H

cfe/trunk/lib/AST/ASTImporter.cpp

Decl *ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
   DeclarationName Name;
   SourceLocation Loc;
   NamedDecl *ToD;
   if (ImportDeclParts(D, DC, LexicalDC, Name, ToD, Loc))
     return nullptr;
   if (ToD)
     return ToD;
 
+  const FunctionDecl *FoundWithoutBody = nullptr;
+
   // Try to find a function in our own ("to") context with the same name, same
   // type, and in the same context as the function we're importing.
   if (!LexicalDC->isFunctionOrMethod()) {
     SmallVector<NamedDecl *, 4> ConflictingDecls;
     unsigned IDNS = Decl::IDNS_Ordinary;
     SmallVector<NamedDecl *, 2> FoundDecls;
     DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
     for (unsigned I = 0, N = FoundDecls.size(); I != N; ++I) {
       if (!FoundDecls[I]->isInIdentifierNamespace(IDNS))
         continue;
 
       if (FunctionDecl *FoundFunction = dyn_cast<FunctionDecl>(FoundDecls[I])) {
         if (FoundFunction->hasExternalFormalLinkage() &&
             D->hasExternalFormalLinkage()) {
           if (Importer.IsStructurallyEquivalent(D->getType(), 
                                                 FoundFunction->getType())) {
             // FIXME: Actually try to merge the body and other attributes.
+            const FunctionDecl *FromBodyDecl = nullptr;
+            D->hasBody(FromBodyDecl);
+            if (D == FromBodyDecl && !FoundFunction->hasBody()) {
+              // This function is needed to merge completely.
+              FoundWithoutBody = FoundFunction;
+              break;
+            }
             return Importer.Imported(D, FoundFunction);
           }
 
           // FIXME: Check for overloading more carefully, e.g., by boosting
           // Sema::IsOverload out to the AST library.
 
           // Function overloading is okay in C++.
           if (Importer.getToContext().getLangOpts().CPlusPlus)
Decl *ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
 
   // Set the parameters.
   for (unsigned I = 0, N = Parameters.size(); I != N; ++I) {
     Parameters[I]->setOwningFunction(ToFunction);
     ToFunction->addDeclInternal(Parameters[I]);
   }
   ToFunction->setParams(Parameters);
 
+  if (FoundWithoutBody) {
+    auto *Recent = const_cast<FunctionDecl *>(
+          FoundWithoutBody->getMostRecentDecl());
+    ToFunction->setPreviousDecl(Recent);
+  }
+
   if (usedDifferentExceptionSpec) {
     // Update FunctionProtoType::ExtProtoInfo.
     QualType T = Importer.Import(D->getType());
     if (T.isNull())
       return nullptr;
     ToFunction->setType(T);
   }
 

cfe/trunk/lib/Basic/DiagnosticIDs.cpp

 #include "clang/Basic/DiagnosticCommonKinds.inc"
 #include "clang/Basic/DiagnosticDriverKinds.inc"
 #include "clang/Basic/DiagnosticFrontendKinds.inc"
 #include "clang/Basic/DiagnosticSerializationKinds.inc"
 #include "clang/Basic/DiagnosticLexKinds.inc"
 #include "clang/Basic/DiagnosticParseKinds.inc"
 #include "clang/Basic/DiagnosticASTKinds.inc"
 #include "clang/Basic/DiagnosticCommentKinds.inc"
+#include "clang/Basic/DiagnosticCrossTUKinds.inc"
 #include "clang/Basic/DiagnosticSemaKinds.inc"
 #include "clang/Basic/DiagnosticAnalysisKinds.inc"
 #undef DIAG
 };
 
 static const unsigned StaticDiagInfoSize = llvm::array_lengthof(StaticDiagInfo);
 
 /// GetDiagInfo - Return the StaticDiagInfoRec entry for the specified DiagID,
#define CATEGORY(NAME, PREV) \
   }
 CATEGORY(DRIVER, COMMON)
 CATEGORY(FRONTEND, DRIVER)
 CATEGORY(SERIALIZATION, FRONTEND)
 CATEGORY(LEX, SERIALIZATION)
 CATEGORY(PARSE, LEX)
 CATEGORY(AST, PARSE)
 CATEGORY(COMMENT, AST)
-CATEGORY(SEMA, COMMENT)
+CATEGORY(CROSSTU, COMMENT)
+CATEGORY(SEMA, CROSSTU)
 CATEGORY(ANALYSIS, SEMA)
 #undef CATEGORY
 
   // Avoid out of bounds reads.
   if (ID + Offset >= StaticDiagInfoSize)
     return nullptr;
 
   assert(ID < StaticDiagInfoSize && Offset < StaticDiagInfoSize);

cfe/trunk/lib/CMakeLists.txt

 add_subdirectory(Headers)
 add_subdirectory(Basic)
 add_subdirectory(Lex)
 add_subdirectory(Parse)
 add_subdirectory(AST)
 add_subdirectory(ASTMatchers)
+add_subdirectory(CrossTU)
 add_subdirectory(Sema)
 add_subdirectory(CodeGen)
 add_subdirectory(Analysis)
 add_subdirectory(Edit)
 add_subdirectory(Rewrite)
 if(CLANG_ENABLE_ARCMT)
   add_subdirectory(ARCMigrate)
 endif()

cfe/trunk/lib/CrossTU/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_library(clangCrossTU
+  CrossTranslationUnit.cpp
+
+  LINK_LIBS
+  clangAST
+  clangBasic
+  clangFrontend
+  clangIndex
+  )

cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp

+//===--- CrossTranslationUnit.cpp - -----------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file implements the CrossTranslationUnit interface.
+//
+//===----------------------------------------------------------------------===//
+#include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/AST/ASTImporter.h"
+#include "clang/AST/Decl.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/CrossTU/CrossTUDiagnostic.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendDiagnostic.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Index/USRGeneration.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
+#include <fstream>
+#include <sstream>
+
+namespace clang {
+namespace cross_tu {
+
+namespace {
+// FIXME: This class is will be removed after the transition to llvm::Error.
+class IndexErrorCategory : public std::error_category {
+public:
+  const char *name() const noexcept override { return "clang.index"; }
+
+  std::string message(int Condition) const override {
+    switch (static_cast<index_error_code>(Condition)) {
+    case index_error_code::unspecified:
+      return "An unknown error has occurred.";
+    case index_error_code::missing_index_file:
+      return "The index file is missing.";
+    case index_error_code::invalid_index_format:
+      return "Invalid index file format.";
+    case index_error_code::multiple_definitions:
+      return "Multiple definitions in the index file.";
+    case index_error_code::missing_definition:
+      return "Missing definition from the index file.";
+    case index_error_code::failed_import:
+      return "Failed to import the definition.";
+    case index_error_code::failed_to_get_external_ast:
+      return "Failed to load external AST source.";
+    case index_error_code::failed_to_generate_usr:
+      return "Failed to generate USR.";
+    }
+    llvm_unreachable("Unrecognized index_error_code.");
+  }
+};
+
+static llvm::ManagedStatic<IndexErrorCategory> Category;
+} // end anonymous namespace
+
+char IndexError::ID;
+
+void IndexError::log(raw_ostream &OS) const {
+  OS << Category->message(static_cast<int>(Code)) << '\n';
+}
+
+std::error_code IndexError::convertToErrorCode() const {
+  return std::error_code(static_cast<int>(Code), *Category);
+}
+
+llvm::Expected<llvm::StringMap<std::string>>
+parseCrossTUIndex(StringRef IndexPath, StringRef CrossTUDir) {
+  std::ifstream ExternalFnMapFile(IndexPath);
+  if (!ExternalFnMapFile)
+    return llvm::make_error<IndexError>(index_error_code::missing_index_file,
+                                        IndexPath.str());
+
+  llvm::StringMap<std::string> Result;
+  std::string Line;
+  unsigned LineNo = 1;
+  while (std::getline(ExternalFnMapFile, Line)) {
+    const size_t Pos = Line.find(" ");
+    if (Pos > 0 && Pos != std::string::npos) {
+      StringRef LineRef{Line};
+      StringRef FunctionLookupName = LineRef.substr(0, Pos);
+      if (Result.count(FunctionLookupName))
+        return llvm::make_error<IndexError>(
+            index_error_code::multiple_definitions, IndexPath.str(), LineNo);
+      StringRef FileName = LineRef.substr(Pos + 1);
+      SmallString<256> FilePath = CrossTUDir;
+      if (llvm::sys::path::is_absolute(FileName))
+        FilePath = FileName;
+      else
+        llvm::sys::path::append(FilePath, FileName);
+      Result[FunctionLookupName] = FilePath.str().str();
+    } else
+      return llvm::make_error<IndexError>(
+          index_error_code::invalid_index_format, IndexPath.str(), LineNo);
+    LineNo++;
+  }
+  return Result;
+}
+
+std::string
+createCrossTUIndexString(const llvm::StringMap<std::string> &Index) {
+  std::ostringstream Result;
+  for (const auto &E : Index)
+    Result << E.getKey().str() << " " << E.getValue() << '\n';
+  return Result.str();
+}
+
+CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance &CI)
+    : CI(CI), Context(CI.getASTContext()) {}
+
+CrossTranslationUnitContext::~CrossTranslationUnitContext() {}
+
+std::string CrossTranslationUnitContext::getLookupName(const NamedDecl *ND) {
+  SmallString<128> DeclUSR;
+  bool Ret = index::generateUSRForDecl(ND, DeclUSR);
+  assert(!Ret && "Unable to generate USR");
+  return DeclUSR.str();
+}
+
+/// Recursively visits the function decls of a DeclContext, and looks up a
+/// function based on USRs.
+const FunctionDecl *
+CrossTranslationUnitContext::findFunctionInDeclContext(const DeclContext *DC,
+                                                       StringRef LookupFnName) {
+  assert(DC && "Declaration Context must not be null");
+  for (const Decl *D : DC->decls()) {
+    const auto *SubDC = dyn_cast<DeclContext>(D);
+    if (SubDC)
+      if (const auto *FD = findFunctionInDeclContext(SubDC, LookupFnName))
+        return FD;
+
+    const auto *ND = dyn_cast<FunctionDecl>(D);
+    const FunctionDecl *ResultDecl;
+    if (!ND || !ND->hasBody(ResultDecl))
+      continue;
+    if (getLookupName(ResultDecl) != LookupFnName)
+      continue;
+    return ResultDecl;
+  }
+  return nullptr;
+}
+
+llvm::Expected<const FunctionDecl *>
+CrossTranslationUnitContext::getCrossTUDefinition(const FunctionDecl *FD,
+                                                  StringRef CrossTUDir,
+                                                  StringRef IndexName) {
+  assert(!FD->hasBody() && "FD has a definition in current translation unit!");
+  const std::string LookupFnName = getLookupName(FD);
+  if (LookupFnName.empty())
+    return llvm::make_error<IndexError>(
+        index_error_code::failed_to_generate_usr);
+  llvm::Expected<ASTUnit *> ASTUnitOrError =
+      loadExternalAST(LookupFnName, CrossTUDir, IndexName);
+  if (!ASTUnitOrError)
+    return ASTUnitOrError.takeError();
+  ASTUnit *Unit = *ASTUnitOrError;
+  if (!Unit)
+    return llvm::make_error<IndexError>(
+        index_error_code::failed_to_get_external_ast);
+  assert(&Unit->getFileManager() ==
+         &Unit->getASTContext().getSourceManager().getFileManager());
+
+  TranslationUnitDecl *TU = Unit->getASTContext().getTranslationUnitDecl();
+  if (const FunctionDecl *ResultDecl =
+          findFunctionInDeclContext(TU, LookupFnName))
+    return importDefinition(ResultDecl);
+  return llvm::make_error<IndexError>(index_error_code::failed_import);
+}
+
+void CrossTranslationUnitContext::emitCrossTUDiagnostics(const IndexError &IE) {
+  switch (IE.getCode()) {
+  case index_error_code::missing_index_file:
+    Context.getDiagnostics().Report(diag::err_fe_error_opening)
+        << IE.getFileName() << "required by the CrossTU functionality";
+    break;
+  case index_error_code::invalid_index_format:
+    Context.getDiagnostics().Report(diag::err_fnmap_parsing)
+        << IE.getFileName() << IE.getLineNum();
+  case index_error_code::multiple_definitions:
+    Context.getDiagnostics().Report(diag::err_multiple_def_index)
+        << IE.getLineNum();
+    break;
+  default:
+    break;
+  }
+}
+
+llvm::Expected<ASTUnit *> CrossTranslationUnitContext::loadExternalAST(
+    StringRef LookupName, StringRef CrossTUDir, StringRef IndexName) {
+  // FIXME: The current implementation only supports loading functions with
+  //        a lookup name from a single translation unit. If multiple
+  //        translation units contains functions with the same lookup name an
+  //        error will be returned.
+  ASTUnit *Unit = nullptr;
+  auto FnUnitCacheEntry = FunctionASTUnitMap.find(LookupName);
+  if (FnUnitCacheEntry == FunctionASTUnitMap.end()) {
+    if (FunctionFileMap.empty()) {
+      SmallString<256> IndexFile = CrossTUDir;
+      if (llvm::sys::path::is_absolute(IndexName))
+        IndexFile = IndexName;
+      else
+        llvm::sys::path::append(IndexFile, IndexName);
+      llvm::Expected<llvm::StringMap<std::string>> IndexOrErr =
+          parseCrossTUIndex(IndexFile, CrossTUDir);
+      if (IndexOrErr)
+        FunctionFileMap = *IndexOrErr;
+      else
+        return IndexOrErr.takeError();
+    }
+
+    auto It = FunctionFileMap.find(LookupName);
+    if (It == FunctionFileMap.end())
+      return llvm::make_error<IndexError>(index_error_code::missing_definition);
+    StringRef ASTFileName = It->second;
+    auto ASTCacheEntry = FileASTUnitMap.find(ASTFileName);
+    if (ASTCacheEntry == FileASTUnitMap.end()) {
+      IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+      TextDiagnosticPrinter *DiagClient =
+          new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
+      IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
+      IntrusiveRefCntPtr<DiagnosticsEngine> Diags(
+          new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
+
+      std::unique_ptr<ASTUnit> LoadedUnit(ASTUnit::LoadFromASTFile(
+          ASTFileName, CI.getPCHContainerOperations()->getRawReader(),
+          ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts()));
+      Unit = LoadedUnit.get();
+      FileASTUnitMap[ASTFileName] = std::move(LoadedUnit);
+    } else {
+      Unit = ASTCacheEntry->second.get();
+    }
+    FunctionASTUnitMap[LookupName] = Unit;
+  } else {
+    Unit = FnUnitCacheEntry->second;
+  }
+  return Unit;
+}
+
+llvm::Expected<const FunctionDecl *>
+CrossTranslationUnitContext::importDefinition(const FunctionDecl *FD) {
+  ASTImporter &Importer = getOrCreateASTImporter(FD->getASTContext());
+  auto *ToDecl =
+      cast<FunctionDecl>(Importer.Import(const_cast<FunctionDecl *>(FD)));
+  assert(ToDecl->hasBody());
+  assert(FD->hasBody() && "Functions already imported should have body.");
+  return ToDecl;
+}
+
+ASTImporter &
+CrossTranslationUnitContext::getOrCreateASTImporter(ASTContext &From) {
+  auto I = ASTUnitImporterMap.find(From.getTranslationUnitDecl());
+  if (I != ASTUnitImporterMap.end())
+    return *I->second;
+  ASTImporter *NewImporter =
+      new ASTImporter(Context, Context.getSourceManager().getFileManager(),
+                      From, From.getSourceManager().getFileManager(), false);
+  ASTUnitImporterMap[From.getTranslationUnitDecl()].reset(NewImporter);
+  return *NewImporter;
+}
+
+} // namespace cross_tu
+} // namespace clang

cfe/trunk/test/Analysis/func-mapping-test.cpp

+// RUN: %clang_func_map %s -- | FileCheck %s
+
+int f(int) {
+  return 0;
+}
+
+// CHECK: c:@F@f#I#

cfe/trunk/test/CMakeLists.txt

list(APPEND CLANG_TEST_DEPS
   clang-format
   c-index-test diagtool
   clang-tblgen
   clang-offload-bundler
   clang-import-test
   clang-rename
   clang-refactor
   clang-diff
+  clang-func-mapping
   )
   
 if(CLANG_ENABLE_STATIC_ANALYZER)
   list(APPEND CLANG_TEST_DEPS
     clang-check
     )
 endif()
 

cfe/trunk/test/lit.cfg.py

config.substitutions.append( ('%clang_cc1',
                               '%s -cc1 -internal-isystem %s -nostdsysteminc'
                               % (config.clang, builtin_include_dir)) )
 config.substitutions.append( ('%clang_cpp', ' ' + config.clang +
                               ' --driver-mode=cpp '))
 config.substitutions.append( ('%clang_cl', ' ' + config.clang +
                               ' --driver-mode=cl '))
 config.substitutions.append( ('%clangxx', ' ' + config.clang +
                               ' --driver-mode=g++ '))
+config.substitutions.append( ('%clang_func_map', ' ' + lit.util.which('clang-func-mapping', config.environment['PATH']) + ' ') )
 config.substitutions.append( ('%clang', ' ' + config.clang + ' ') )
 config.substitutions.append( ('%test_debuginfo',
                              ' ' + config.llvm_src_root +  '/utils/test_debuginfo.pl ') )
 config.substitutions.append( ('%itanium_abi_triple',
                              llvm_config.make_itanium_abi_triple(config.target_triple)) )
 config.substitutions.append( ('%ms_abi_triple',
                              llvm_config.make_msabi_triple(config.target_triple)) )
 config.substitutions.append( ('%resource_dir', builtin_include_dir) )

cfe/trunk/tools/CMakeLists.txt

 
 if(CLANG_ENABLE_ARCMT)
   add_clang_subdirectory(arcmt-test)
   add_clang_subdirectory(c-arcmt-test)
 endif()
 
 if(CLANG_ENABLE_STATIC_ANALYZER)
   add_clang_subdirectory(clang-check)
+  add_clang_subdirectory(clang-func-mapping)
   add_clang_subdirectory(scan-build)
   add_clang_subdirectory(scan-view)
 endif()
 
 # We support checking out the clang-tools-extra repository into the 'extra'
 # subdirectory. It contains tools developed as part of the Clang/LLVM project
 # on top of the Clang tooling platform. We keep them in a separate repository
 # to keep the primary Clang repository small and focused.
 # It also may be included by LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR.
 add_llvm_external_project(clang-tools-extra extra)
 
 # libclang may require clang-tidy in clang-tools-extra.
 add_clang_subdirectory(libclang)

cfe/trunk/tools/clang-func-mapping/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+  asmparser
+  support
+  mc
+  )
+
+add_clang_executable(clang-func-mapping
+  ClangFnMapGen.cpp
+  )
+
+target_link_libraries(clang-func-mapping
+  clangAST
+  clangBasic
+  clangCrossTU
+  clangFrontend
+  clangIndex
+  clangTooling
+  )
+
+install(TARGETS clang-func-mapping
+  RUNTIME DESTINATION bin)

cfe/trunk/tools/clang-func-mapping/ClangFnMapGen.cpp

+//===- ClangFnMapGen.cpp -----------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--------------------------------------------------------------------===//
+//
+// Clang tool which creates a list of defined functions and the files in which
+// they are defined.
+//
+//===--------------------------------------------------------------------===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/GlobalDecl.h"
+#include "clang/AST/Mangle.h"
+#include "clang/AST/StmtVisitor.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Index/USRGeneration.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Signals.h"
+#include <sstream>
+#include <string>
+#include <vector>
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::cross_tu;
+using namespace clang::tooling;
+
+static cl::OptionCategory ClangFnMapGenCategory("clang-fnmapgen options");
+
+class MapFunctionNamesConsumer : public ASTConsumer {
+public:
+  MapFunctionNamesConsumer(ASTContext &Context) : Ctx(Context) {}
+
+  ~MapFunctionNamesConsumer() {
+    // Flush results to standard output.
+    llvm::outs() << createCrossTUIndexString(Index);
+  }
+
+  virtual void HandleTranslationUnit(ASTContext &Ctx) {
+    handleDecl(Ctx.getTranslationUnitDecl());
+  }
+
+private:
+  void handleDecl(const Decl *D);
+
+  ASTContext &Ctx;
+  llvm::StringMap<std::string> Index;
+  std::string CurrentFileName;
+};
+
+void MapFunctionNamesConsumer::handleDecl(const Decl *D) {
+  if (!D)
+    return;
+
+  if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
+    if (FD->isThisDeclarationADefinition()) {
+      if (const Stmt *Body = FD->getBody()) {
+        std::string LookupName = CrossTranslationUnitContext::getLookupName(FD);
+        const SourceManager &SM = Ctx.getSourceManager();
+        if (CurrentFileName.empty()) {
+          CurrentFileName =
+              SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
+          if (CurrentFileName.empty())
+            CurrentFileName = "invalid_file";
+        }
+
+        switch (FD->getLinkageInternal()) {
+        case ExternalLinkage:
+        case VisibleNoLinkage:
+        case UniqueExternalLinkage:
+          if (SM.isInMainFile(Body->getLocStart()))
+            Index[LookupName] = CurrentFileName;
+        default:
+          break;
+        }
+      }
+    }
+  }
+
+  if (const auto *DC = dyn_cast<DeclContext>(D))
+    for (const Decl *D : DC->decls())
+      handleDecl(D);
+}
+
+class MapFunctionNamesAction : public ASTFrontendAction {
+protected:
+  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+                                                 llvm::StringRef) {
+    std::unique_ptr<ASTConsumer> PFC(
+        new MapFunctionNamesConsumer(CI.getASTContext()));
+    return PFC;
+  }
+};
+
+static cl::extrahelp CommonHelp(CommonOptionsParser::HelpMessage);
+
+int main(int argc, const char **argv) {
+  // Print a stack trace if we signal out.
+  sys::PrintStackTraceOnErrorSignal(argv[0], false);
+  PrettyStackTraceProgram X(argc, argv);
+
+  const char *Overview = "\nThis tool collects the USR name and location "
+                         "of all functions definitions in the source files "
+                         "(excluding headers).\n";
+  CommonOptionsParser OptionsParser(argc, argv, ClangFnMapGenCategory,
+                                    cl::ZeroOrMore, Overview);
+
+  ClangTool Tool(OptionsParser.getCompilations(),
+                 OptionsParser.getSourcePathList());
+  Tool.run(newFrontendActionFactory<MapFunctionNamesAction>().get());
+  return 0;
+}

cfe/trunk/tools/diagtool/DiagnosticNames.cpp

 
 // FIXME: Is it worth having two tables, especially when this one can get
 // out of sync easily?
 static const DiagnosticRecord BuiltinDiagnosticsByID[] = {
 #define DIAG(ENUM,CLASS,DEFAULT_MAPPING,DESC,GROUP,               \
              SFINAE,NOWERROR,SHOWINSYSHEADER,CATEGORY)            \
   { #ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t) },
 #include "clang/Basic/DiagnosticCommonKinds.inc"
+#include "clang/Basic/DiagnosticCrossTUKinds.inc"
 #include "clang/Basic/DiagnosticDriverKinds.inc"
 #include "clang/Basic/DiagnosticFrontendKinds.inc"
 #include "clang/Basic/DiagnosticSerializationKinds.inc"
 #include "clang/Basic/DiagnosticLexKinds.inc"
 #include "clang/Basic/DiagnosticParseKinds.inc"
 #include "clang/Basic/DiagnosticASTKinds.inc"
 #include "clang/Basic/DiagnosticCommentKinds.inc"
 #include "clang/Basic/DiagnosticSemaKinds.inc"

cfe/trunk/unittests/CMakeLists.txt

 add_subdirectory(Driver)
 if(CLANG_ENABLE_STATIC_ANALYZER)
   add_subdirectory(Analysis)
   add_subdirectory(StaticAnalyzer)
   add_subdirectory(Frontend)
 endif()
 add_subdirectory(ASTMatchers)
 add_subdirectory(AST)
+add_subdirectory(CrossTU)
 add_subdirectory(Tooling)
 add_subdirectory(Format)
 add_subdirectory(Rewrite)
 add_subdirectory(Sema)
 add_subdirectory(CodeGen)
 # FIXME: libclang unit tests are disabled on Windows due
 # to failures, mostly in libclang.VirtualFileOverlay_*.
 if(NOT WIN32 AND CLANG_TOOL_LIBCLANG_BUILD) 
   add_subdirectory(libclang)
 endif()
 add_subdirectory(Rename)

cfe/trunk/unittests/CrossTU/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+  Support
+  )
+
+add_clang_unittest(CrossTUTests
+  CrossTranslationUnitTest.cpp
+  )
+
+target_link_libraries(CrossTUTests
+  clangAST
+  clangBasic
+  clangCrossTU
+  clangFrontend
+  clangTooling
+  )

cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp

+//===- unittest/Tooling/CrossTranslationUnitTest.cpp - Tooling unit tests -===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Config/llvm-config.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/ToolOutputFile.h"
+#include "gtest/gtest.h"
+#include <cassert>
+
+namespace clang {
+namespace cross_tu {
+
+namespace {
+
+class CTUASTConsumer : public clang::ASTConsumer {
+public:
+  explicit CTUASTConsumer(clang::CompilerInstance &CI, bool *Success)
+      : CTU(CI), Success(Success) {}
+
+  void HandleTranslationUnit(ASTContext &Ctx) {
+    const TranslationUnitDecl *TU = Ctx.getTranslationUnitDecl();
+    const FunctionDecl *FD = nullptr;
+    for (const Decl *D : TU->decls()) {
+      FD = dyn_cast<FunctionDecl>(D);
+      if (FD && FD->getName() == "f")
+        break;
+    }
+    assert(FD && FD->getName() == "f");
+    bool OrigFDHasBody = FD->hasBody();
+
+    // Prepare the index file and the AST file.
+    int ASTFD;
+    llvm::SmallString<256> ASTFileName;
+    ASSERT_FALSE(
+        llvm::sys::fs::createTemporaryFile("f_ast", "ast", ASTFD, ASTFileName));
+    llvm::tool_output_file ASTFile(ASTFileName, ASTFD);
+
+    int IndexFD;
+    llvm::SmallString<256> IndexFileName;
+    ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
+                                                    IndexFileName));
+    llvm::tool_output_file IndexFile(IndexFileName, IndexFD);
+    IndexFile.os() << "c:@F@f#I# " << ASTFileName << "\n";
+    IndexFile.os().flush();
+    EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
+
+    StringRef SourceText = "int f(int) { return 0; }\n";
+    // This file must exist since the saved ASTFile will reference it.
+    int SourceFD;
+    llvm::SmallString<256> SourceFileName;
+    ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("input", "cpp", SourceFD,
+                                                    SourceFileName));
+    llvm::tool_output_file SourceFile(SourceFileName, SourceFD);
+    SourceFile.os() << SourceText;
+    SourceFile.os().flush();
+    EXPECT_TRUE(llvm::sys::fs::exists(SourceFileName));
+
+    std::unique_ptr<ASTUnit> ASTWithDefinition =
+        tooling::buildASTFromCode(SourceText, SourceFileName);
+    ASTWithDefinition->Save(ASTFileName.str());
+    EXPECT_TRUE(llvm::sys::fs::exists(ASTFileName));
+
+    // Load the definition from the AST file.
+    llvm::Expected<const FunctionDecl *> NewFDorError =
+        CTU.getCrossTUDefinition(FD, "", IndexFileName);
+    EXPECT_TRUE((bool)NewFDorError);
+    const FunctionDecl *NewFD = *NewFDorError;
+
+    *Success = NewFD && NewFD->hasBody() && !OrigFDHasBody;
+  }
+
+private:
+  CrossTranslationUnitContext CTU;
+  bool *Success;
+};
+
+class CTUAction : public clang::ASTFrontendAction {
+public:
+  CTUAction(bool *Success) : Success(Success) {}
+
+protected:
+  std::unique_ptr<clang::ASTConsumer>
+  CreateASTConsumer(clang::CompilerInstance &CI, StringRef) override {
+    return llvm::make_unique<CTUASTConsumer>(CI, Success);
+  }
+
+private:
+  bool *Success;
+};
+
+} // end namespace
+
+TEST(CrossTranslationUnit, CanLoadFunctionDefinition) {
+  bool Success = false;
+  EXPECT_TRUE(tooling::runToolOnCode(new CTUAction(&Success), "int f(int);"));
+  EXPECT_TRUE(Success);
+}
+
+TEST(CrossTranslationUnit, IndexFormatCanBeParsed) {
+  llvm::StringMap<std::string> Index;
+  Index["a"] = "b";
+  Index["c"] = "d";
+  Index["e"] = "f";
+  std::string IndexText = createCrossTUIndexString(Index);
+
+  int IndexFD;
+  llvm::SmallString<256> IndexFileName;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
+                                                  IndexFileName));
+  llvm::tool_output_file IndexFile(IndexFileName, IndexFD);
+  IndexFile.os() << IndexText;
+  IndexFile.os().flush();
+  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
+  llvm::Expected<llvm::StringMap<std::string>> IndexOrErr =
+      parseCrossTUIndex(IndexFileName, "");
+  EXPECT_TRUE((bool)IndexOrErr);
+  llvm::StringMap<std::string> ParsedIndex = IndexOrErr.get();
+  for (const auto &E : Index) {
+    EXPECT_TRUE(ParsedIndex.count(E.getKey()));
+    EXPECT_EQ(ParsedIndex[E.getKey()], E.getValue());
+  }
+  for (const auto &E : ParsedIndex)
+    EXPECT_TRUE(Index.count(E.getKey()));
+}
+
+} // end namespace cross_tu
+} // end namespace clang