Add index-while-building support to Clang

[jkorous]

Summary

Adds a new -index-store-path option that causes Clang to additionally collect and output source code indexing information to the supplied path. This is done by wrapping the FrontendAction otherwise setup by the invocation with a WrappingIndexRecordAction. This action simply delegates to the wrapped action, but additionally multiplexes in its own IndexASTConsumer to collect symbol information from the AST and tracks the source file and module dependencies of the translation unit (via the IndexDependencyProvider class).

When the action completes, it then writes this information out to the supplied index store path in the form of a unit file, which stores the dependency information of the translation unit, and record files, that store the symbol and symbol occurrences seen in each source file. These are written out in the LLVM Bitstream format.

For a better (and more detailed) description of these changes, see the design document at: https://docs.google.com/document/d/1cH2sTpgSnJZCkZtJl1aY-rzy4uGPcrI-6RrUpdATO2Q/edit?usp=sharing

and the mailing list discussion 'RFC: Adding index-while-building support to Clang'

[arphaman]

lib/Index/IndexRecordHasher.cpp

103 ↗

(On Diff #120916)

Should you hash the return type as well?

204 ↗

(On Diff #120916)

You can use Qualifiers::Const here or make your own enum instead of raw constants.

[nathawes]

Thanks @arphaman! I'll work through your comments and update.

include/clang/Index/IndexDataStoreSymbolUtils.h

13 ↗

(On Diff #120916)

They're used by IndexRecordWriter below to convert from libIndex's representation of things to the index store's.

[nathawes]

lib/Index/IndexRecordHasher.cpp

103 ↗

(On Diff #120916)

The return type doesn't affect the function's USR, so there's no need to consider it when hashing the function decl. The hashing here is happening per decl occurrence (source offset + role set + Decl) collected by the index AST walker, so changing the return type will still change the record hash when any decl occurrences it contains are hashed in.

[nathawes]

Based on @arphaman's feedback:

• Pulled the index store related diagnostics out into their own category/diagnostic group
• Removed the CLANG_PROJECT_INDEX_PATH env var check.
• Swapped "/" used in a few places as a separator/root with the equivalent llvm::sys::path call.
• Fixed the typo/convention/documentation issues and simplifications pointed out so far

[malaperle]

lib/Index/IndexUnitWriter.cpp

212 ↗

(On Diff #121833)

extra semi-colon (noticed this warning while compiling)

[malaperle]

lib/Index/IndexingAction.cpp

567

As a first attempt, I tried to use index::createIndexDataRecordingAction in combination with ASTUnit::LoadFromCompilerInvocationAction but one problem is that right before it calls EndSourceFileAction in LoadFromCompilerInvocationAction, it calls transferASTDataFromCompilerInstance which means that the SourceManager in CompilerInstance is nulled out as it gets "transfered" to the AST. So this line crashes in this case. To be fair, at this point I don't need the ASTUnit so I can look at executing the action differently, but I thought I'd point it out!

[malaperle]

lib/Index/IndexRecordWriter.cpp

155 ↗

(On Diff #121833)

I'm getting quite a bit of those while indexing Clangd, it looks like it comes from some LLVM/Support headers:
Index: Duplicate USR! c:@N@std@ST>2#NI#Nb@__try_lock_impl
Index: Duplicate USR! c:@N@llvm@ST>1#T@DenseMapInfo
Index: Duplicate USR! c:@N@llvm@ST>1#T@isPodLike
Index: Duplicate USR! c:@N@llvm@N@detail@ST>1#T@unit
Index: Duplicate USR! c:@N@llvm@ST>2#T#T@format_provider
Index: Duplicate USR! c:@N@llvm@ST>2#T#T@format_provider
Index: Duplicate USR! c:@N@llvm@ST>1#T@PointerLikeTypeTraits
Index: Duplicate USR! c:@N@llvm@ST>1#T@simplify_type
Index: Duplicate USR! c:@N@std@ST>1#T@atomic
Index: Duplicate USR! c:@N@llvm@ST>1#T@isPodLike
Index: Duplicate USR! c:@N@llvm@ST>1#T@DenseMapInfo

I think it would be good to have the file name at least in the log. I also assume those duplication are issues that would have to be fixed in USRGenerator (i.e. in separate patches) ?

[malaperle]

include/indexstore/IndexStoreCXX.h

75 ↗

(On Diff #118854)

I know this is an old revision but I thought I should ask for the future patch... Would it be possible to not use "blocks"? This will affect portability of the code. I'm not familiar with blocks but I would think it would be possible to replace with C++11 lambdas, or something else that's standard. I was just playing around with this code and since I used GCC, it did not work.

[ioeric]

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

[nathawes]

In D39050#920451, @ioeric wrote:

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

Thanks for taking a look @ioeric! I'll have a go at splitting it further.

Some comments/ideas:

• The lack of tests is a bit concerning.

I moved all the code for reading the index store data into a separate patch (to come after this one) in order to slim this one down for review, and most of the tests went with it because they're based around reading and dumping the stored data for FileCheck. The original version of this patch has them all (https://reviews.llvm.org/D39050?id=118854). The ones that remain here are just those checking that the unit/record files are written out and that the hashing mechanism is producing distinct record files when the symbolic content of the source file changes.

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

• 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!

[ioeric]

• 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!

[akyrtzi]

Hey Eric,

In D39050#921748, @ioeric wrote:

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

As Nathan mentioned, we believe the indexing action, as it exists in the trunk, is decoupled enough to be useful, for example Marc was already able to use it and write out the indexing data in a completely different format for his fork of clangd. Of course, we are definitely interested in any additional refactorings that would structure things better and we are eager to see and discuss follow-up patches from anyone that is interested in improving the code, but could we treat this as potential follow-up improvements ?

We are eager to provide the functionality so others can start experimenting with it; I'd propose that we discuss ideas for refactoring of the code as follow-up, what do you think ? Getting the initial functionality in and iterating on it, while getting more experience with applying it on various use-cases, is a common operating mindset of the llvm/clang projects.

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!

Nathan will look into making using blocks optional, providing additional function pointer+context APIs where appropriate and having the common implementation using lambdas.
For the APPLE specific parts it, the only specific darwin-specific part is the part using FSEvents, the other 'if (APPLE)' checks can likely be removed. We would generally need help from people with linux expertise to provide the 'FSEvents' equivalent functionality but this is a small part of the overall feature, it's not important for getting the index-while-building data.

But these things are not part of the current patch, we can discuss again with the follow-up patches that will contain those things.

Index-while-build is awesome! I'm looking forward to your patches!

[ioeric]

In D39050#922597, @akyrtzi wrote:

Hey Eric,

In D39050#921748, @ioeric wrote:

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

As Nathan mentioned, we believe the indexing action, as it exists in the trunk, is decoupled enough to be useful, for example Marc was already able to use it and write out the indexing data in a completely different format for his fork of clangd. Of course, we are definitely interested in any additional refactorings that would structure things better and we are eager to see and discuss follow-up patches from anyone that is interested in improving the code, but could we treat this as potential follow-up improvements ?

We are eager to provide the functionality so others can start experimenting with it; I'd propose that we discuss ideas for refactoring of the code as follow-up, what do you think ? Getting the initial functionality in and iterating on it, while getting more experience with applying it on various use-cases, is a common operating mindset of the llvm/clang projects.

To be honest, I want this functionality to get in as much as you do, and I'm more than happy to prioritize the code review for it :) But the current patch size makes the reviewing really hard (e.g. I would never have caught the BLOCK issues hadn't I tried running the original patch myself). I'm not sure if it's really a common practice to check in a big chunk of code without careful code review and leave potential improvements as followups. I'm sure @klimek would have thoughts about this.

If the index action is already flexible enough, would you mind splitting the code for the index action out so that we can start reviewing it? Given that the current patch has very few tests, I guess it wouldn't be too much worse to split out the action without proper test.

[akyrtzi]

To be honest, I want this functionality to get in as much as you do, and I'm more than happy to prioritize the code review for it :) But the current patch size makes the reviewing really hard (e.g. I would never have caught the BLOCK issues hadn't I tried running the original patch myself). I'm not sure if it's really a common practice to check in a big chunk of code without careful code review and leave potential improvements as followups. I'm sure @klimek would have thoughts about this.

To be clear, I didn't mean to imply we don't want careful code review, we are really happy for people to point out issues. For example the building problems on linux are serious issues that we will fix and we are grateful for your feedback!

If the index action is already flexible enough, would you mind splitting the code for the index action out so that we can start reviewing it? Given that the current patch has very few tests, I guess it wouldn't be too much worse to split out the action without proper test.

To clarify, the index action Nathan and I are referring to, is the indexing action that exists currently in trunk and is the source of the index symbols, feeding index symbols to an abstract IndexDataConsumer. See here: https://llvm.org/svn/llvm-project/cfe/trunk/include/clang/Index/IndexingAction.h
This is what Marc used to get the index symbols and store them in his own format. Tests for this functionality are in: https://llvm.org/svn/llvm-project/cfe/trunk/test/Index/Core/

[ioeric]

If the index action is already flexible enough, would you mind splitting the code for the index action out so that we can start reviewing it? Given that the current patch has very few tests, I guess it wouldn't be too much worse to split out the action without proper test.

To clarify, the index action Nathan and I are referring to, is the indexing action that exists currently in trunk and is the source of the index symbols, feeding index symbols to an abstract IndexDataConsumer. See here: https://llvm.org/svn/llvm-project/cfe/trunk/include/clang/Index/IndexingAction.h
This is what Marc used to get the index symbols and store them in his own format. Tests for this functionality are in: https://llvm.org/svn/llvm-project/cfe/trunk/test/Index/Core/

Ah, sorry, I was referring to IndexRecordAction and its friends (record readers/writers). I didn't notice the newly added index action added and really didn't mean to ask you to refactor the existing code. Apologies for the miscommunication!

What I wanted to proposed is that we could decouple reading/writing of record/unit from the bit file format, so that the record output is not tied to a single output format (e.g. bit format, directory-based) and thus make the compiler more flexible. This might already be the case, but it's not really easy to tell from the current patch...

[malaperle]

Hi! I got a bit further in my experiment in integrating this in Clangd. I put some comments (in the first more complete revision). But since the scope of this patch changed, if you feel like we should take the discussions elsewhere, please let me know! Thanks!

include/indexstore/IndexStoreCXX.h

84 ↗

(On Diff #118854)

From what I understand, this returns the beginning of the occurrence. It would be useful to also have the end of the occurrence. From what I tested in Xcode, when you do "Find Selected Symbol in Workspace", it highlights the symbol name in yellow in the list of matches, so it mush use that LineCol then highlight the matching name. This is works in many situations but others occurrences won't have the name of the symbol. For example:
"MyClass o1, o2;"
If I use "Find Selected Symbol in Workspace" on MyClass constructor, if won't be able to highlight o1 and o2
Do you think it would be possible to add that (EndLineCol)? If not, how would one go about extending libindexstore in order to add additional information per occurrence? It is not obvious to me so far.
We also need other things, for example in the case of definitions, we need the full range of the definition so that users can "peek" at definitions in a pop-up. Without storing the end of the definition in the index, we would need to reparse the file.

374 ↗

(On Diff #118854)

As part of this dependency tracking mechanism, I haven't found that it could provide information about about the files including a specific header. So given a header (or source file in odd cases), I do not see a way to get all the files that would need to be reindexed if that header changed. Is that something you achieve outside the index? Or perhaps this is something I missed in the code.

377 ↗

(On Diff #118854)

Could there be a bit of explanation about what's a File dependency versus record and unit? All units and records are file dependencies, right? Are there any files that are neither records or units?

[nathawes]

Thanks for the feedback @malaperle!

include/indexstore/IndexStoreCXX.h

84 ↗

(On Diff #118854)

Our approach to related locations (e.g. name, signature, body, and doc comment start/end locs) has been to not include them in the index and derive them from the start location later. There's less data to collect and write out during the build that way, and deriving the other locations isn't that costly usually, as in most cases you 1) don't need to type check or even preprocess to get the related locations, as with finding the end of o1 and o2 in your example, and 2) usually only need to derive locations for a single or small number of occurrences, like when 'peeking' at a definition.

Are there cases where you think this approach won't work/perform well enough for the indexer-backed queries clangd needs to support?

374 ↗

(On Diff #118854)

The unit files store the path of the header/source files they depend on as 'File' dependencies. So any unit file with 'File' dependency on header/source file that was modified may need to be re-indexed.

To support finding which specific files include or are included by a given header (rather than which units somehow transitively include it) we also store the file-to-file inclusions in the unit file (retrieved via IndexUnitReader's foreachInclude method below).

377 ↗

(On Diff #118854)

I'll rename this to SourceFile and add some comments to explain. Unit file dependencies separate the source dependencies into 'File' dependencies and 'Record' dependencies. The 'File' dependencies track the paths of the header/source files seen in the translation unit, while the 'Record' dependencies track which record files have the symbolic content seen in those source files – the header/source file path doesn't appear anywhere in the record file. This separation lets us depend on a single record file corresponding to multiple source files (e.g. when two source files have the same symbolic content), and on a single source file corresponding multiple record files (e.g. when a single header is included multiple times with different preprocessor contexts changing its symbolic content).

[ioeric]

First round of comments. Mostly around indexing actions and file records; I haven't started reviewing the data writing and storing code. I think it might make sense to split the index writing and storing logics into a separate patch, which should be possible if writeUnitData is abstracted into an interface.

include/clang/Frontend/CompilerInstance.h

188

It might make sense to define an alias for std::function<std::unique_ptr<FrontendAction>(const FrontendOptions &opts, std::unique_ptr<FrontendAction> action)>, which is used multiple times.

include/clang/Frontend/FrontendOptions.h

262

It might make sense to also have documentations for these options here.

include/indexstore/IndexStoreCXX.h

84 ↗

(On Diff #118854)

I agree that we should try to keep the serialized symbol size minimal, but I think it's worthwhile to also store end locations for occurrences because 1) it's cheap, 2) it's not necessary easy to compute without parsing for occurrences like a::b::c or a::b<X>, 3) it would be useful for many LSP use cases.

lib/Frontend/CompilerInstance.cpp

1176

nit: no braces around one liners.

lib/FrontendTool/ExecuteCompilerInvocation.cpp

170

Could you comment on what this does? The Act above is already wrapped. Why do we need setGenModuleActionWrapper to createIndexDataRecordingAction again? Also, createIndexDataRecordingAction doesn't seem related to GenModule.

lib/Index/FileIndexRecord.cpp

24

Why?

38

Please comment when this would happen.

40

Why do we need Decls to be sorted by offset? If we want this for printing, it might make sense to just do a sort there.

lib/Index/FileIndexRecord.h

25

Please add documentation.

42

Is this clang-formatted? You might want to run git-clang-format on the whole patch.

lib/Index/IndexingAction.cpp

289

Again, you don't need the full IndexingContext and RecordOptions here.

299

Note that getDecomposedExpansionLoc can also return invalid decomposed loc.

313

Do we want better error handling here?

332

Please provide documentation.

509

Can we get this state from the base class instead of maintaining a another state, which seems to be identical?

529

Just StringRef BuildNumber = RepositoryPath;

706

Please provide a brief documentation for this class.

708

Again, it doesn't seem necessary for this class to have information about all record options. It seems that you only need RecordSystemDependencies here.

718

readability nit: avoid using auto if the return type is short to spell but hard to infer from the value expression. Same else where.

767

Could you add a comment explaining why we are not allowing searching.

774

It's a bit worrying that IndexDataRecorder and IndexContext reference each other. If you only need some information from the IndexingContext, simply pass it into Recorder. In this case, I think you only need the SourceManager from the ASTContext in the recorder to calculate whether a file is a system header. I see you also cache result of IndexingContext::isSystemFile in the indexing context, but I think it would be more sensible for the callers to handle caching for this call.

776

nit: no braces around one liners.

784

nit: redundant empty line

842

Just auto pair = getIndexOptionsFromFrontendOptions(FEOpts); and then use pair.first and pair.second?

Same below.

lib/Index/IndexingContext.h

60

Please define the scope of this class to avoid throwing random states into it, which usually happens to a "context" class.

[malaperle]

include/indexstore/IndexStoreCXX.h

84 ↗

(On Diff #118854)

There's a few reason I think it's better to store the end loc.

• When doing "find references", computing the end loc of each occurrence will be costly. Imagine having thousands of occurrences and for each of them having to run logic to find the end of the occurrence.
• The AST and preprocessor are the best tools I know to figure out the proper end loc. Not using them means having to write a mini-preprocessor with some knowledge about the language semantics to cover some cases.

MyClass |o1, o2;

Here, I have to stop at the comma. So it's basically take any alpha-numerical character, right?

bool operator<(const Foo&, const Foo&)
Ret operator()(Params ...params)

No, in those cases, we have to take < and the first ().

• In the case of body start/end locations, similarly, it can be non-trivial.

void foo() {
  if (0) {
  }
}

We have to count the balanced { } until we finish the body.

#define FUNC_BODY {\
\
}
void foo() FUNC_BODY

Oops, where's the body? We need another special logic for this, etc.

I think overall, it puts a lot of burden on the client of libIndexStore, burden that would be much more work and more inaccurate than using the AST/Preprocessor while indexing.

374 ↗

(On Diff #118854)

Thanks! I'll play around with this a bit more with this new information.

377 ↗

(On Diff #118854)

It's more clear now, thanks!

[akyrtzi]

@malaperle, to clarify we are not suggesting that you write your own parser, the suggestion is to use clang in 'fast-scan' mode to get the structure of the declarations of a single file, see CXTranslationUnit_SingleFileParse (along with enabling skipping of bodies). We have found clang is super fast when you only try to get the structure of a file like this. We can make convenient APIs to provide the syntactic structure of declarations based on their location.

But let's say we added the end-loc, is it enough ? If you want to implement the 'peek the definition' like Eclipse, then it is not enough, you also need to figure out if there are documentation comments associated with the declaration and also show those. Also what if you want to highlight the type signature of a function, then just storing the location of the closing brace of its body is not enough. There can be any arbitrary things you may want to get from the structure of the declaration (e.g. the parameter ranges), but we could provide an API to gather any syntactic structure info you may want.

I would encourage you to try CXTranslationUnit_SingleFileParse|CXTranslationUnit_SkipFunctionBodies, you will be pleasantly surprised with how fast this mode is. The c-index-test option is -single-file-parse.

[nathawes]

lib/FrontendTool/ExecuteCompilerInvocation.cpp

170

It's to wrap any GenerateModuleActions that get created as needed when/if Act ends up loading any modules, so that we output index data for them too. I'll add a comment.

lib/Index/FileIndexRecord.cpp

40

It's mostly for when we hash them, so that ordering doesn't change the hash, but it's also for printing. The IndexASTConsumer doesn't always report symbol occurrences in source order, due to the preprocessor and a few other cases.
We can sort them when the IndexRecordDataConsumer's finish() is called rather than as they're added to avoid the copying from repeated insert calls if that's the concern.

lib/Index/IndexingAction.cpp

509

I don't see this state in either base class (WrapperFrontendAction and IndexRecordActionBase). WrappingIndexAction and WrappingIndexRecordAction both have this, though. Were you thinking a new intermediate common base class between them and WrapperFrontendAction?

774

Good point. The IndexingContext was actually already calling IsSystemFile before it calls IndexDataRecorder's handleDeclOccurrence and handleModuleOccurrence anyway, so I'll change it to pass that through as an extra param and remove IndexDataRecorder's dependency on the IndexingContext.

[nathawes]

Worked through the comments from @ioeric and split the code for writing out the collected indexing data into a separate patch.

[malaperle]

In D39050#948500, @akyrtzi wrote:

@malaperle, to clarify we are not suggesting that you write your own parser, the suggestion is to use clang in 'fast-scan' mode to get the structure of the declarations of a single file, see CXTranslationUnit_SingleFileParse (along with enabling skipping of bodies). We have found clang is super fast when you only try to get the structure of a file like this.

Thank you, that sounds very useful. I will try that and get some measurements.

We can make convenient APIs to provide the syntactic structure of declarations based on their location.

Perhaps just for the end-loc since it's pretty much guaranteed to be needed by everyone. But if it's very straightforward, perhaps that's not needed. I'll try and see.

But let's say we added the end-loc, is it enough ? If you want to implement the 'peek the definition' like Eclipse, then it is not enough, you also need to figure out if there are documentation comments associated with the declaration and also show those. Also what if you want to highlight the type signature of a function, then just storing the location of the closing brace of its body is not enough. There can be any arbitrary things you may want to get from the structure of the declaration (e.g. the parameter ranges), but we could provide an API to gather any syntactic structure info you may want.

That's a very good point. I guess in the back of my mind, I have the worry that one cannot extend what is stored, either for a different performance trade-off or for additional things. The fact that both clang and clangd have to agree on the format so that index-while-building can be used seems to make it inherently not possible to extend. But perhaps it's better to not overthink this for now.

[ioeric]

Thanks a lot for the changes! Some more comments inlined.

Please mark addressed comments as done so that reviewers could know what to look :) Thanks!

include/clang/Frontend/CompilerInstance.h

187

nit: LLVM variable names start with upper-case letters.

include/clang/Index/IndexingAction.h

34

This should be removed?

Some forward declarations above are not used as well.

lib/Driver/Job.cpp

293

Could you share this code with line 278 above, which already has a nice comment?

lib/Index/FileIndexRecord.cpp

40

I would leave the sorting to the point where records are hashed to avoid making the record stateful. Consider changing getDeclOccurrences to getOccurrencesSortedByOffset; this should make the behavior more explicit.

lib/Index/FileIndexRecord.h

51

s/isSystem/IsSystem/

Also, I wonder if we can filter out system decls proactively and avoid creating file index record for them. We could also avoid propogating IsSystem here.

lib/Index/IndexingAction.cpp

370

IsSystemFileCache &SysrootPath? What is this parameter?

459

Please document this class. This can be easily confused with IndexActionBase which has a similar name. Same for IndexAction/IndexRecordAction and WrappingIndexRecordAction/WrappingIndexRecordAction. I think these pairs share (especially the wrapping actions) some common logics and could probably be merged.

485

This does a lot of stuff... please document the behavior!

509

I thought this could be a state in the WrapperFrontendAction since both derived classes maintain this state, but after a closer look, this seems to depend on both base classes. I'm not a big fun of maintaining states in multi-stage classes (e.g. FrontendAction), which could be confusing and hard to follow; I think IndexRecordActionBase::finish(...) should be able to handle the case where no index consumer is created (i.e. no record/dependency/... is collected).

Also, IndexRecordActionBase (and the existing IndexActionBase ) should really be a component instead of a base class since none of its methods is virtual.

577

nit: no need for braces. Same below.

589

In the previous patch, writeUnitData does several things including handling modules, dependencies, includes and index records, as well as writing data. It might make sense to add an abstract class (UnitDataCollector?) that defines interfaces which make these behavior more explicit. We can then have users pass in an implementation via createIndexDataRecordingAction which would also decouple the data collection from data storage in the library.

624

I'm a bit nervous about propagating the entire FrontendOptions into the index library. I would simply expose getIndexOptionsFromFrontendOptions and have callers parse FrontendOptions and pass in only index-related options.

lib/Index/IndexingContext.h

41

This name is really confusing... Is* is usually used for booleans. Simply call this SystemFileCache.

53

How does this affect the existing cached results? Do you need to invalidate them?

63

I think it would be more straightforward to have context own the cache. If setSysrootPath is the problem, it might make sense to propagate it via the context or, if necessary, create a new cache when a new SysrootPath is set.

[nathawes]

Thanks for taking another look @ioeric – I'll work through your comments and update.

[nathawes]

lib/Index/FileIndexRecord.h

51

If the -index-ignore-system-symbols flag is set system decls are filtered out in IndexingContext::handleDeclOccurrence and aren't reported to the IndexDataConsumer, so FileIndexRecords won't be created. The IsSystem here is for clients that want index data for system files, but want to be able to distinguish them from regular files.

[nathawes]

I've refactored the indexing/dependency data collection out from the writing with the new IndexUnitDataConsumer class, and made other smaller changes to address the feedback from @ioeric.

[nathawes]

Fix out of date header comment in FileIndexData.h

[ioeric]

Thanks a lot for further cleaning up the patch! It is now much easier to review. I really appreciate it!

Some more comments on the public APIs and the layering of classes. There are a lot of helper classes in the implementation, so I think it's important to get a clear layering so that they could be easily understood by future contributors :)

Also, with the IndexUnitDataConsumer abstraction, it seems to be feasible now to add some unit tests for createUnitIndexingAction. With all the recent major changes, I think it's important that we have some degree of testing to make sure components actually work together.

include/clang/Frontend/CompilerInstance.h

187

opts and action are still lower-case.

include/clang/Index/DeclOccurrence.h

38 ↗

(On Diff #127412)

Nit: indentation.

Tip: git-clang-format against the diff base can format all changed lines in your patch.

include/clang/Index/IndexUnitDataConsumer.h

1 ↗

(On Diff #127568)

IIUC, this is the index data for a translation unit, as opposed to an AST. If so, consider calling this UnitIndexDataConsumer to match (AST)IndexDataConsumer which is parallel to this. We might want to rename them to be either index::UnitDataConsumer vs index::ASTDataConsumer or index::UnitIndexDataConsumer vs index::ASTIndexDataConsumer . I am inclined to the first pair as index is already implied in the namespace.

67 ↗

(On Diff #127568)

Comment? Why do we actually need this?

include/clang/Index/IndexingAction.h

48

We are now mixing functionalities for Unit indexing and AST indexing actions in the same file. We might want to separate these into two headers e..g UnitIndexingAction.h and ASTIndexingAction.h. This would make it easier for users to find the right functions :)

65

Please add documentation for each field. It's not trivial what each field is for, especially some fields seem to be optional and some seem to be mutually exclusive.

66

These pointers suggest the life time of this struct is tied to some other struct, which makes the struct look a bit dangerous to use. Should we also carry a reference or a smart pointer to the underlying object that keeps these pointers valid? Would it be a CompilerInstance (guessing from IndexUnitDataConsumerFactory )?

95

What is the intended user of this function? It's unclear how users could obtain a ConsumerFactory (i.e. UnitDetails) without the functionalities in UnitDataConsumerActionImpl .

(Also see comment in the implementation of createIndexDataRecordingAction.)

109

This is likely only useful for compiler invocation. I would put it in the compiler invocation code.

lib/Driver/Job.cpp

211

nit: Comment should start with an overview of what the function does.

Returns a directory path that is ...

Also, consider calling this getDirAdjacentToModCache. buildDir can be ambiguous.

220

Please clang-format the code. Without indentation, this looks like an no-op statement.

lib/Index/IndexingAction.cpp

93

Use class for interfaces.

103

Does CI here have to be the same instance as the one in createIndexASTConsumer ? Might worth documenting.

141

nit: Move this after Impl->createIndexASTConsumer(CI).

Do we need to reset this flag? Calling CreateASTConsumer multiple times on the same instance seems to be allowed?

229

This seems to be related to files. Maybe FileIndexDataCollector?

236

override

240

Simply begin, if the class is called FileIndexDataCollector .

Similar below to match iterator naming convention.

251

I think this should be public as this is still implementing IndexDataConsumer.

309

I'd simply do:

if FileIncludeFilter  == UnitIndexingOptions::FileIncludeFilterKind::UserOnly)
  if (isSystem...)
    return;

323

Same here. This should be public

340

The naming convention for the callback interfaces is forEach* e.g. forEachFileDependency.

s/visitor/Callback/
(same below).

344

forEachInclude

347

forEachModuleImport

355

This is two classes in one, which is difficult to understand. Could you split it into FileIndexDependencyCollector and FileIndexDependencyProvider and have FileIndexDependencyCollector returns a provider on finish (e.g. Provider consume();; you might want to copy/move the collected data into the provider). It would be easier to justify the behavior (e.g. what happens when you access the provider while collector is still working?)

359

What does Entries contain? What files are added?

487

Instead of passing ParentUnitConsumer, consider checking the Mod before calling the function.

490

Non-factory static method is often a code smell. Any reason not to make these static methods private members? With that, you wouldn't need to pass along so many parameters. You could make them const if you don't want members to be modified.

497

Why is this overload public while others are private? Aren't they all used only in this class?

517

Any reason to close the anonymous namespace here? Shouldn't outlined definitions of UnitDataConsumerActionImpl's methods also in the anonymous namespace?

744

I think the inheritance of IndexUnitDataConsumer and the creation of factory should be in user code (e.g. implementation for on-disk persist-index-data should come from the compiler invocation code ExecuteCompilerInvocation.cpp or at least a separate file in the library that compiler invocation can use), and the user should only use createUnitIndexingAction by providing a factory. Currently, createUnitIndexingAction and createIndexDataRecordingAction are mostly identical except for the code that implements IndexUnitDataConsumer and creates the factory.

The current createIndexDataRecordingAction would probably only used by the compiler invocation, and we can keep the generalized createUnitIndexingAction in the public APIs.

751

The UnitInfo is ignored? What do we actually need it for?

755

Base doesn't seem to be a very meaningful name here.

[ioeric]

include/clang/Index/IndexUnitDataConsumer.h

1 ↗

(On Diff #127568)

Sorry, asking you to also rename IndexDataConsumer is probably too much and out of the scope of this patch. I'm fine with UnitIndexDataConsumer or UnitDataConsumer or something similar for now without touching IndexDataConsumer :)

[ioeric]

(I think I forgot to update the patch status :)

[nathawes]

@ioeric I should have an updated patch up shortly with your inline comments addressed + new tests. Thanks again for reviewing!

include/clang/Index/IndexUnitDataConsumer.h

67 ↗

(On Diff #127568)

From here, my understanding is that it's an optimization to avoid the vtable being included in multiple translation units. I'm not sure if that's actually a problem, I was just following IndexDataConsumer's lead. Added a comment.

include/clang/Index/IndexingAction.h

95

Sorry, I'm not sure what you mean here. Users shouldn't need to know anything about UnitDataConsumerActionImpl, they just need to provide a lambda/function reference that takes a CompilerInstance& and a UnitDetails and returns an IndexUnitDataConsumer (or UnitIndexDataConsumer once I rename it). This gets called once per translation unit to get a distinct data consumer for each unit, i.e. for the main translation unit as well as for each of its dependent modules that the main unit's data consumer says should be indexed via shouldIndexModuleDependency(...).

109

There's another public index:: API for writing out index data for individual clang module files in the follow up patch that takes a RecordingOptions and is used externally, from Swift. This function's useful on the Swift side to get the RecordingOptions from FrontendOptions it has already set up.

lib/Index/IndexingAction.cpp

141

Oops. Yes, we do :-)

490

Sorry, there's missing context – they're used from another public API that's in the follow-up patch. I'll bring that over and make these top-level static functions, since they don't belong exclusively to IndexDataConsumerActionImpl.

497

Same as above – this is called from a public index:: API in the follow-up patch.

744

IndexUnitDataRecorder here is just a stub I added when I split the patch up – the follow-up revision has it in a separate file. I'll move the separate files to this patch and stub out the method bodies with TODOs instead.

I've made createIndexDataRecordingAction call createUnitIndexingAction to remove the duplication, and pulled it, RecordingOptions and getRecordingOptionsFromFrontendOptions to a new header (RecordingAction.h) that ExecuteComilerInvocation.cpp uses. Does that sound ok?

751

It should be passed to IndexUnitDataRecorder to write out info about the unit itself. This was just me splitting the patch badly.

[nathawes]

• Applied the various refactorings suggested by @ioeric
• Extended c-index-test with a new option to print out the collected unit indexing data, and
• Added tests for the unit indexing functionality using the new option
• Fixed formatting

[ioeric]

Nice! Thanks for making the refactoring and adding tests! I think this is good to go now.

I'm not very familiar with code outside of the index library (Driver, Basic etc), but the changes seem reasonable to me. Feel free to get another pair of eyes for them ;)

include/clang/Index/RecordingAction.h

42 ↗

(On Diff #130496)

Add a FIXME that this is not implemented yet.

lib/Index/IndexingAction.cpp

744

Sounds good. Thanks for the explanation!

[ioeric]

I'm wondering if there is any further plan for this? ;)

[malaperle]

In D39050#1004937, @ioeric wrote:

I'm wondering if there is any further plan for this? ;)

I'd like to comment on the amount of data that will be stored but that can be done outside this review. I still have a few things to figure out before reaching a conclusion.

[nathawes]

@ioeric I'm working on a few other priorities over the next few weeks, sorry, but should get back to this relatively soon after that.
I would just land it, but I expect some downstream breakage I want to make sure I have time to fix.

@malaperle Sounds good – I'll keep an eye out for it!

[malaperle]

In D39050#949185, @malaperle wrote:

In D39050#948500, @akyrtzi wrote:

@malaperle, to clarify we are not suggesting that you write your own parser, the suggestion is to use clang in 'fast-scan' mode to get the structure of the declarations of a single file, see CXTranslationUnit_SingleFileParse (along with enabling skipping of bodies). We have found clang is super fast when you only try to get the structure of a file like this.

Thank you, that sounds very useful. I will try that and get some measurements.

We can make convenient APIs to provide the syntactic structure of declarations based on their location.

Perhaps just for the end-loc since it's pretty much guaranteed to be needed by everyone. But if it's very straightforward, perhaps that's not needed. I'll try and see.

But let's say we added the end-loc, is it enough ? If you want to implement the 'peek the definition' like Eclipse, then it is not enough, you also need to figure out if there are documentation comments associated with the declaration and also show those. Also what if you want to highlight the type signature of a function, then just storing the location of the closing brace of its body is not enough. There can be any arbitrary things you may want to get from the structure of the declaration (e.g. the parameter ranges), but we could provide an API to gather any syntactic structure info you may want.

That's a very good point. I guess in the back of my mind, I have the worry that one cannot extend what is stored, either for a different performance trade-off or for additional things. The fact that both clang and clangd have to agree on the format so that index-while-building can be used seems to make it inherently not possible to extend. But perhaps it's better to not overthink this for now.

I did a bit more of experimenting. For the end-loc, I changed my prototype so that the end-loc is not stored in the index but rather computed "on the fly" using SourceManager and Lexer only. For my little benchmark, I used the LLVM/Clang/Clangd code base which I queried for all references of "std" (the namespace) which is around 46K references in the index.

With end-loc in index: 3.45s on average (20 samples)
With end-loc computed on the fly: 11.33s on average (20 samples)
I also tried with Xcode but without too much success: it took about 30 secs to reach 45K results and then carried on for a long time and hung (although I didn't try to leave it for hours to see if it finished).

From my perspective, it seems that the extra time is quite substantial and it doesn't seem worth to save an integer per occurrence in this case.

For computing the start/end-loc of function bodies, I tried the SingleFileParseMode and SkipFunctionBodies separately ( as a start). The source I use this on looks like this:

#include "MyClass.h"

MyClass::MyClass() {
}

void MyClass::doOperation() {
}

With SingleFileParseMode, I get several errors:

MyClass.cpp:5:1: error: use of undeclared identifier 'MyClass'
MyClass.cpp:8:6: error: use of undeclared identifier 'MyClass'

Then I cannot obtain any Decl* at the position of doOperation. With SingleFileParseMode, I'm also a bit weary that not processing headers will result in many inaccuracies. From our perspective, we are more wiling to sacrifice disk space in order to have more accuracy and speed. For comparison, the index I worked with containing all end-loc for occurrences and also function start/end is 201M for LLVM/Clang/Clangd which is small to us.

With SkipFunctionBodies alone, I can get the Decl* but FunctionDecl::getSourceRange() doesn't include the body, rather, it stops after the arguments.
It would be very nice if we could do this cheaply but it doesn't seem possible with those two flags alone. What did you have in mind for implementing an "API to gather any syntactic structure info" ?

[malaperle]

In D39050#1021204, @malaperle wrote:

For computing the start/end-loc of function bodies, I tried the SingleFileParseMode and SkipFunctionBodies separately ( as a start). The source I use this on looks like this:

Given the discussion in https://reviews.llvm.org/D44247, I think we can do without the start/end-loc of function bodies and try some heuristics client-side. We can always revisit this later if necessary.

However, for the end-loc of occurrences, would you be OK with this being added? I think it would be a good compromise in terms of performance, simplicity and index size.

[nathawes]

In D39050#1036249, @malaperle wrote:

In D39050#1021204, @malaperle wrote:

For computing the start/end-loc of function bodies, I tried the SingleFileParseMode and SkipFunctionBodies separately ( as a start). The source I use this on looks like this:

Given the discussion in https://reviews.llvm.org/D44247, I think we can do without the start/end-loc of function bodies and try some heuristics client-side. We can always revisit this later if necessary.

However, for the end-loc of occurrences, would you be OK with this being added? I think it would be a good compromise in terms of performance, simplicity and index size.

@malaperle Just to clarify, what's the particular end-loc we're talking about here? e.g. for a function call, would this be the end of the function's name, or the closing paren?
For the end of the name, couldn't this be derived from the start loc + symbol name length (barring token pastes and escaped new lines in the middle of identifiers, which hopefully aren't too common)?
I can see the value for the closing paren though.

@akyrtzi Are the numbers from Marc-Andre's experiment what you'd expect to see and is there anything else to try? I'm not familiar with those modes at all to comment, sorry. I assume any API to gather syntactic structure info would be based on those modes, right?

[malaperle]

In D39050#1036394, @nathawes wrote:

@malaperle Just to clarify, what's the particular end-loc we're talking about here? e.g. for a function call, would this be the end of the function's name, or the closing paren?
For the end of the name, couldn't this be derived from the start loc + symbol name length (barring token pastes and escaped new lines in the middle of identifiers, which hopefully aren't too common)?
I can see the value for the closing paren though.

I mean the end of the name referencing the symbol, so that it can be highlighted properly when using the "find references in workspace" feature. There are cases where the name of the symbol itself is not present, for example "MyClass o1, o2;" (o1 and o2 reference the constructor), references to overloaded operators, etc.

[nathawes]

In D39050#1037008, @malaperle wrote:

In D39050#1036394, @nathawes wrote:

@malaperle Just to clarify, what's the particular end-loc we're talking about here? e.g. for a function call, would this be the end of the function's name, or the closing paren?
For the end of the name, couldn't this be derived from the start loc + symbol name length (barring token pastes and escaped new lines in the middle of identifiers, which hopefully aren't too common)?
I can see the value for the closing paren though.

I mean the end of the name referencing the symbol, so that it can be highlighted properly when using the "find references in workspace" feature. There are cases where the name of the symbol itself is not present, for example "MyClass o1, o2;" (o1 and o2 reference the constructor), references to overloaded operators, etc.

Ah, I see – thanks! I was thinking all occurrences whose symbol name didn't actually appear at their location were marked with SymbolRole::Implicit, but that only seems to be true for the ObjC index data.

[akyrtzi]

Hey Marc,

The fact that both clang and clangd have to agree on the format so that index-while-building can be used seems to make it inherently not possible to extend

I don't think "not possible to extend" is quite correct, we can make it so that the format allows optional data to be recorded.

On the topic of recording the end-loc, I agree it's not much data overall, but it will be useful to examine the uses closely and to figure out whether it's really required and whether it is at the same time inadequate for other uses.

I changed my prototype so that the end-loc is not stored in the index but rather computed "on the fly" using SourceManager and Lexer only.

I assume you used SingleFileParseMode+SkipFunctionBodies for this, right ?

For my little benchmark, I used the LLVM/Clang/Clangd code base which I queried for all references of "std" (the namespace) which is around 46K references in the index.

This is an interesting use case, and I can say we have some experience because Xcode has this functionality without requiring the end-loc for every reference.
So what it does is that it has a 'name' to look for (say 'foo' for the variable foo) and if it finds the name in the location then it highlights, otherwise if it doesn't find it (e.g. because it is an implicit reference) then it points to the location but doesn't highlight something. The same thing happens for operator overloads (the operators get highlighted at the reference location).
For implicit references it's most likely there's nothing to highlight so the end-loc will most likely be empty anyway (or same as start-loc ?) to indicate an empty range.

With SingleFileParseMode, I get several errors:

Good point, the parser definitely needs recovery improvements in C++.

With SkipFunctionBodies alone, I can get the Decl* but FunctionDecl::getSourceRange() doesn't include the body

This seems strange, there's an EndRangeLoc field that should have been filled in, not exactly sure if it is a bug or omission.

Going back to the topic of what use cases end-loc covers, note that it still seems inadequate for peek-definition functionality. You can't set it to body-end loc (otherwise occurrences highlighting would highlight the whole body which I think is undesirable) and you still need to include doc-comments if they exist.

[malaperle]

In D39050#1037796, @akyrtzi wrote:

Hey Marc,

The fact that both clang and clangd have to agree on the format so that index-while-building can be used seems to make it inherently not possible to extend

I don't think "not possible to extend" is quite correct, we can make it so that the format allows optional data to be recorded.

That would be good. How would one go about asking Clang to generate this extra information? Would a Clang Plugin be suitable for this? I don't know much about those but perhaps that could be one way to extent the basic behavior of "-index_store_path" in this way?

I changed my prototype so that the end-loc is not stored in the index but rather computed "on the fly" using SourceManager and Lexer only.

I assume you used SingleFileParseMode+SkipFunctionBodies for this, right ?

No, sorry the end-locs I meant there is for occurrences. Only the lexer was needed to get the end of the token. So for "MyClass o1, o2;" o1 and o2 get highlighted as references to the MyClass constructor.

For my little benchmark, I used the LLVM/Clang/Clangd code base which I queried for all references of "std" (the namespace) which is around 46K references in the index.

This is an interesting use case, and I can say we have some experience because Xcode has this functionality without requiring the end-loc for every reference.
So what it does is that it has a 'name' to look for (say 'foo' for the variable foo) and if it finds the name in the location then it highlights, otherwise if it doesn't find it (e.g. because it is an implicit reference) then it points to the location but doesn't highlight something.

I think it's useful to highlight something even when the name is not there. For example in "MyClass o1, o2;" it feels natural that o1 and o2 would get highlighted.

The same thing happens for operator overloads (the operators get highlighted at the reference location).

It does? I can only seem to do a textual search. For example, if I look at "FileId::operator<", if I right-click in the middle of "operator<" and do "Find selected symbol in workspace", it seems to start a text based search because there are many results that are semantically unrelated.

For implicit references it's most likely there's nothing to highlight so the end-loc will most likely be empty anyway (or same as start-loc ?) to indicate an empty range.

I think for those cases the end of the token is probably suitable. Can you give examples which implicit references you have in mind? Maybe another one (other than the constructor mentioned above) could be a function call like "passMeAStdString(MyStringRef)", here the "operator std::string" would be called and MyStringRef could be highlighted, I think it would make sense to the user that is gets called by passing this parameter by seeing the highlight.

Going back to the topic of what use cases end-loc covers, note that it still seems inadequate for peek-definition functionality. You can't set it to body-end loc (otherwise occurrences highlighting would highlight the whole body which I think is undesirable) and you still need to include doc-comments if they exist.

I think maybe I wasn't clear, I was thinking about two end-locs: end-locs of occurrences and end-locs of bodies. The end-loc of occurrences would be used for highlight when searching for all occurrences and the end-loc for bodies would be used for the peek definition. I think we can disregard end-locs of bodies for now.

[akyrtzi]

That would be good. How would one go about asking Clang to generate this extra information? Would a Clang Plugin be suitable for this?

That's an interesting idea that we could explore, but I don't have much experience with that mechanism to comment on.

Only the lexer was needed to get the end of the token

Ok, that's interesting, not sure why Xcode is so fast to highlight, did you reuse same SourceManager/Lexer/buffers for occurrences from same file ? We'd definitely add the end-loc if we cannot come up with a mechanism to highlight fast enough without it.

I think it's useful to highlight something even when the name is not there. For example in "MyClass o1, o2;" it feels natural that o1 and o2 would get highlighted.

To clarify, even with implicit references the start loc points to something. In this case the implicit references can have start locs for the o1 and o2 identifiers and the end result for the UI will be the same (o1 and o2 get highlighted) even without having end-locs for all references.

It does? I can only seem to do a textual search.

The example I tried is the following. If you could file a bug report for the test case that did not work as you expected it would be much appreciated!

class Something1 {
public:
    Something1() {}
    ~Something1() {}
    operator int() {
        return 0;
    }

    friend int operator <<(Something1 &p, Something1 &p2) {
        return 0;
    }
};

void foo1(Something1 p1, Something1 p2) {
    p1 << p2;
    p1 << p2;
}

here the "operator std::string" would be called and MyStringRef could be highlighted

Even without end-loc, the start loc could point to MyStringRef and you could highlight it.

[malaperle]

In D39050#1040501, @akyrtzi wrote:

That would be good. How would one go about asking Clang to generate this extra information? Would a Clang Plugin be suitable for this?

That's an interesting idea that we could explore, but I don't have much experience with that mechanism to comment on.

Only the lexer was needed to get the end of the token

Ok, that's interesting, not sure why Xcode is so fast to highlight, did you reuse same SourceManager/Lexer/buffers for occurrences from same file ? We'd definitely add the end-loc if we cannot come up with a mechanism to highlight fast enough without it.

I don't think Xcode is quite fast, it's about 10 times slower (although I'm not sure it really finished) than when I use my branch that has the end-loc. I would try end-locs in Xcode if I could, to compare :) So I don't really know where the bottleneck is in Xcode. Comparing oranges to oranges, it's 4 times slower without end-locs compared to with end-locs on my branch. I does use the same SourceManager for the 46K references and I verified that it uses the same buffers, etc.
I'll put the numbers here again for readability.

For my little benchmark, I used the LLVM/Clang/Clangd code base which I queried for all references of "std" (the namespace) which is around 46K references in the index.

With end-loc in index: 3.45s on average (20 samples)
With end-loc computed on the fly: 11.33s on average (20 samples)
I also tried with Xcode but without too much success: it took about 30 secs to reach 45K results and then carried on for a long time and hung (although I didn't try to leave it for hours to see if it finished).

I think it's useful to highlight something even when the name is not there. For example in "MyClass o1, o2;" it feels natural that o1 and o2 would get highlighted.

To clarify, even with implicit references the start loc points to something. In this case the implicit references can have start locs for the o1 and o2 identifiers and the end result for the UI will be the same (o1 and o2 get highlighted) even without having end-locs for all references.

It's the same but slower. IMO, the trade off is not great. It's entirely subjective but I think 4-10 times slower in order to save an integer per occurrence is not worth it from my point of view.

Even without end-loc, the start loc could point to MyStringRef and you could highlight it.

(Same here, it's doable but faster if already in the index.)

It does? I can only seem to do a textual search.

The example I tried is the following. If you could file a bug report for the test case that did not work as you expected it would be much appreciated!

Sure, I'll give that a try and isolate it as much as I can. BTW, does it work for you on the LLVM code base?

[nathawes]

Updated to apply on top-of-tree.

[gribozavr]

include/clang/Frontend/FrontendOptions.h

262

Please end comments with a period.

265

Would it make more sense to flip this boolean to positive? "IndexIncludeSystemSymbols"?

lib/Index/IndexingAction.cpp

112

Please don't duplicate the information from the signature in comments. No need to say that this function returns an IndexASTConsumer (twice, in the first sentence and in the \returns clause), the code already says that.

Also, "The compiler instance used to process the input" does not mean much to me either.

147

No semicolon.

156

No semicolon.

160

No semicolon.

250

Please don't duplicate type information from the signature in the comment.

258

I don't understand... this is not really the user-specified output file.

278

Please don't duplicate type information from the signature in the comment.

lib/Index/IndexingContext.h

40

Please add a period at the end of the comment.

44

DirEntries => IsSystemDirEntry?

46

Triple slashes for doc comments.

46

Unclear how a boolean can keep track of the last check.

Did you mean "Whether the file is a system file or not. This value is a cache." If so, please rename the variable to something like IsSystemFileCache.

test/Index/Core/index-source.mm

2 ↗

(On Diff #153190)

No need to specify check-prefixes=CHECK.

test/Index/Core/index-unit.mm

1 ↗

(On Diff #153190)

This test is very difficult to read... it is just a dump of random internal data structures... what do you think about converting it to a unit test?

[mgrang]

lib/Index/FileIndexData.cpp

31 ↗

(On Diff #153190)

Please use range-based llvm::sort instead of std::sort:

llvm::sort(Sorted);

See https://llvm.org/docs/CodingStandards.html#beware-of-non-deterministic-sorting-order-of-equal-elements

[jkorous]

It's time to officially abandon these patches in favor of new push for upstreaming index-while-building.

Current reviews in progress
https://reviews.llvm.org/D58749
https://reviews.llvm.org/D58418

RFC
http://lists.llvm.org/pipermail/cfe-dev/2019-February/061432.html

[jkorous]

I'll address comments for this patch in the new set of patches.

@gribozavr I haven't put up this part of code for the new round of review yet. I will keep this on mind.

@mgrang This already landed in edbbe470f66 as clang/lib/Index/FileIndexRecord.cpp but luckily the implementation isn't using sort() at all. Thanks for pointing this out anyway!

[akyrtzi]

include/clang/Frontend/FrontendOptions.h

265

@jkorous I noticed this name can be misleading, it may seem as if what this does is "avoid indexing system symbol occurrences" but what it actually does is "avoid indexing symbol occurrences from system files". We should rename it to "IndexIgnoreSystemHeaders" or "IndexIncludeSystemHeaders" per Dmitri's suggestion.

include/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/Index/IndexDiagnostic.h"
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "clang/Serialization/SerializationDiagnostic.h"
 #include "clang/Tooling/Refactoring/RefactoringDiagnostic.h"
 
 namespace clang {
 template <size_t SizeOfStr, typename FieldType>

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(Index)
 clang_diag_gen(Lex)
 clang_diag_gen(Parse)
 clang_diag_gen(Refactoring)
 clang_diag_gen(Sema)
 clang_diag_gen(Serialization)
 clang_tablegen(DiagnosticGroups.inc -gen-clang-diag-groups
   SOURCE Diagnostic.td
   TARGET ClangDiagnosticGroups)

include/clang/Basic/Diagnostic.td

 // 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 "DiagnosticIndexKinds.td"
 include "DiagnosticLexKinds.td"
 include "DiagnosticParseKinds.td"
 include "DiagnosticRefactoringKinds.td"
 include "DiagnosticSemaKinds.td"
 include "DiagnosticSerializationKinds.td"
 

include/clang/Basic/DiagnosticGroups.td

 def MethodSignatures : DiagGroup<"method-signatures">;
 def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">;
 def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">;
 def MismatchedTags : DiagGroup<"mismatched-tags">;
 def MissingFieldInitializers : DiagGroup<"missing-field-initializers">;
 def ModuleBuild : DiagGroup<"module-build">;
 def ModuleConflict : DiagGroup<"module-conflict">;
 def ModuleFileExtension : DiagGroup<"module-file-extension">;
+def IndexStore : DiagGroup<"index-store">;
 def NewlineEOF : DiagGroup<"newline-eof">;
 def Nullability : DiagGroup<"nullability">;
 def NullabilityDeclSpec : DiagGroup<"nullability-declspec">;
 def NullabilityInferredOnNestedType : DiagGroup<"nullability-inferred-on-nested-type">;
 def NullableToNonNullConversion : DiagGroup<"nullable-to-nonnull-conversion">;
 def NullabilityCompletenessOnArrays : DiagGroup<"nullability-completeness-on-arrays">;
 def NullabilityCompleteness : DiagGroup<"nullability-completeness",
                                         [NullabilityCompletenessOnArrays]>;

include/clang/Basic/DiagnosticIDs.h

enum {
       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,
       DIAG_SIZE_REFACTORING   = 1000,
+      DIAG_SIZE_INDEX         =  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_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_START_REFACTORING   = DIAG_START_ANALYSIS      + DIAG_SIZE_ANALYSIS,
-      DIAG_UPPER_LIMIT         = DIAG_START_REFACTORING   + DIAG_SIZE_REFACTORING
+      DIAG_START_INDEX         = DIAG_START_REFACTORING   + DIAG_SIZE_REFACTORING,
+      DIAG_UPPER_LIMIT         = DIAG_START_INDEX         + DIAG_SIZE_INDEX,
     };
 
     class CustomDiagInfo;
 
     /// \brief All of the diagnostics that can be emitted by the frontend.
     typedef unsigned kind;
 
     // Get typedefs for common diagnostics.

include/clang/Basic/DiagnosticIndexKinds.td

+//==--- DiagnosticIndexKinds.td - indexing diagnostics --------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+//===----------------------------------------------------------------------===//
+// Indexing Diagnostics
+//===----------------------------------------------------------------------===//
+
+let Component = "Index" in {
+
+let CategoryName = "Index Store Issue" in {
+
+def err_index_store_dir_create_failed : Error<"failed creating the index store "
+  "directory: %0">;
+def err_index_store_file_status_failed : Error<"failed file status check: %0">;
+def err_index_store_record_write_failed : Error<"failed writing record '%0': "
+  "%1">;
+def err_index_store_unit_write_failed : Error<"failed writing unit data: %0">;
+
+def remark_index_producing_module_file_data : Remark<"producing index data for "
+  "module file '%0'">,
+  InGroup<IndexStore>;
+
+}
+
+} // end of Indexing diagnostics

include/clang/Driver/Job.h

 class InputInfo;
 
 // Re-export this as clang::driver::ArgStringList.
 using llvm::opt::ArgStringList;
 
 struct CrashReportInfo {
   StringRef Filename;
   StringRef VFSPath;
+  StringRef IndexStorePath;
 
-  CrashReportInfo(StringRef Filename, StringRef VFSPath)
-      : Filename(Filename), VFSPath(VFSPath) {}
+  CrashReportInfo(StringRef Filename, StringRef VFSPath,
+                  StringRef IndexStorePath)
+      : Filename(Filename), VFSPath(VFSPath), IndexStorePath(IndexStorePath) {}
 };
 
 /// Command - An executable path/name and argument vector to
 /// execute.
 class Command {
   /// Source - The action which caused the creation of this job.
   const Action &Source;
 

include/clang/Driver/Options.td

 def objcmt_migrate_designated_init : Flag<["-"], "objcmt-migrate-designated-init">, Flags<[CC1Option]>,
   HelpText<"Enable migration to infer NS_DESIGNATED_INITIALIZER for initializer methods">;
 def objcmt_whitelist_dir_path: Joined<["-"], "objcmt-whitelist-dir-path=">, Flags<[CC1Option]>,
   HelpText<"Only modify files with a filename contained in the provided directory path">;
 // The misspelt "white-list" [sic] alias is due for removal.
 def : Joined<["-"], "objcmt-white-list-dir-path=">, Flags<[CC1Option]>,
     Alias<objcmt_whitelist_dir_path>;
 
+def index_store_path : Separate<["-"], "index-store-path">, Flags<[CC1Option]>,
+  HelpText<"Enable indexing with the specified data store path">;
+def index_ignore_system_symbols : Flag<["-"], "index-ignore-system-symbols">, Flags<[CC1Option]>,
+  HelpText<"Ignore symbols from system headers">;
+def index_record_codegen_name : Flag<["-"], "index-record-codegen-name">, Flags<[CC1Option]>,
+  HelpText<"Record the codegen name for symbols">;
+
 // Make sure all other -ccc- options are rejected.
 def ccc_ : Joined<["-"], "ccc-">, Group<internal_Group>, Flags<[Unsupported]>;
 
 // Standard Options
 
 def _HASH_HASH_HASH : Flag<["-"], "###">, Flags<[DriverOption, CoreOption]>,
     HelpText<"Print (but do not run) the commands to run for this compilation">;
 def _DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>,

include/clang/Frontend/CompilerInstance.h

class CompilerInstance : public ModuleLoader {
   /// If the output doesn't support seeking (terminal, pipe). we switch
   /// the stream to a buffer_ostream. These are the buffer and the original
   /// stream.
   std::unique_ptr<llvm::raw_fd_ostream> NonSeekStream;
 
   /// The list of active output files.
   std::list<OutputFile> OutputFiles;
 
+  typedef std::function<std::unique_ptr<FrontendAction>(
+      const FrontendOptions &opts, std::unique_ptr<FrontendAction> action)>
+      ActionWrapperTy;
+
+  /// \brief An optional callback function used to wrap any
+  /// GenerateModuleActions created and executed when loading modules.
+  ActionWrapperTy GenModuleActionWrapper;
+
   CompilerInstance(const CompilerInstance &) = delete;
   void operator=(const CompilerInstance &) = delete;
 public:
   explicit CompilerInstance(
       std::shared_ptr<PCHContainerOperations> PCHContainerOps =
           std::make_shared<PCHContainerOperations>(),
       MemoryBufferCache *SharedPCMCache = nullptr);
   ~CompilerInstance() override;
public:
   bool hadModuleLoaderFatalFailure() const {
     return ModuleLoader::HadFatalFailure;
   }
 
   GlobalModuleIndex *loadGlobalModuleIndex(SourceLocation TriggerLoc) override;
 
   bool lookupMissingImports(StringRef Name, SourceLocation TriggerLoc) override;
 
+  void setGenModuleActionWrapper(ActionWrapperTy Wrapper) {
+    GenModuleActionWrapper = Wrapper;
+  };
+
+  ActionWrapperTy getGenModuleActionWrapper() const {
+    return GenModuleActionWrapper;
+  }
+
   void addDependencyCollector(std::shared_ptr<DependencyCollector> Listener) {
     DependencyCollectors.push_back(std::move(Listener));
   }
 
   void setExternalSemaSource(IntrusiveRefCntPtr<ExternalSemaSource> ESS);
 
   MemoryBufferCache &getPCMCache() const { return *PCMCache; }
 };
 
 } // end namespace clang
 
 #endif

include/clang/Frontend/FrontendOptions.h

ObjCMT_MigrateAll = (ObjCMT_Literals | ObjCMT_Subscripting |
                          ObjCMT_MigrateDecls | ObjCMT_PropertyDotSyntax)
   };
   unsigned ObjCMTAction;
   std::string ObjCMTWhiteListPath;
 
   std::string MTMigrateDir;
   std::string ARCMTMigrateReportOut;
 
+  /// The path to write index data to
+  std::string IndexStorePath;
+  /// Whether to ignore system files when writing out index data
+  unsigned IndexIgnoreSystemSymbols : 1;
+  /// Whether to include the codegen name of symbols in the index data
+  unsigned IndexRecordCodegenName : 1;
+
   /// The input files and their types.
   std::vector<FrontendInputFile> Inputs;
 
   /// When the input is a module map, the original module map file from which
   /// that map was inferred, if any (for umbrella modules).
   std::string OriginalModuleMap;
 
   /// The output file, if any.
public:
   FrontendOptions() :
     DisableFree(false), RelocatablePCH(false), ShowHelp(false),
     ShowStats(false), ShowTimers(false), ShowVersion(false),
     FixWhatYouCan(false), FixOnlyWarnings(false), FixAndRecompile(false),
     FixToTemporaries(false), ARCMTMigrateEmitARCErrors(false),
     SkipFunctionBodies(false), UseGlobalModuleIndex(true),
     GenerateGlobalModuleIndex(true), ASTDumpDecls(false), ASTDumpLookups(false),
     BuildingImplicitModule(false), ModulesEmbedAllFiles(false),
-    IncludeTimestamps(true), ARCMTAction(ARCMT_None),
-    ObjCMTAction(ObjCMT_None), ProgramAction(frontend::ParseSyntaxOnly)
+    IncludeTimestamps(true), ARCMTAction(ARCMT_None), ObjCMTAction(ObjCMT_None),
+    IndexIgnoreSystemSymbols(false), IndexRecordCodegenName(false),
+    ProgramAction(frontend::ParseSyntaxOnly)
   {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file
   /// extension. For example, "c" would return InputKind::C.
   ///
   /// \return The input kind for the extension, or InputKind::Unknown if the
   /// extension is not recognized.
   static InputKind getInputKindForExtension(StringRef Extension);
 };
 
 }  // end namespace clang
 
 #endif

include/clang/Index/IndexDataConsumer.h

public:
   virtual ~IndexDataConsumer() {}
 
   virtual void initialize(ASTContext &Ctx) {}
 
   /// \returns true to continue indexing, or false to abort.
   virtual bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
                                    ArrayRef<SymbolRelation> Relations,
                                    FileID FID, unsigned Offset,
-                                   ASTNodeInfo ASTNode);
+                                   bool IsInSystemFile, ASTNodeInfo ASTNode);
 
   /// \returns true to continue indexing, or false to abort.
   virtual bool handleMacroOccurence(const IdentifierInfo *Name,
                                     const MacroInfo *MI, SymbolRoleSet Roles,
-                                    FileID FID, unsigned Offset);
+                                    FileID FID, unsigned Offset,
+                                    bool IsInSystemFile);
 
   /// \returns true to continue indexing, or false to abort.
   virtual bool handleModuleOccurence(const ImportDecl *ImportD,
-                                     SymbolRoleSet Roles,
-                                     FileID FID, unsigned Offset);
+                                     SymbolRoleSet Roles, FileID FID,
+                                     unsigned Offset, bool IsInSystemFile);
 
   virtual void finish() {}
 
 private:
   virtual void _anchor();
 };
 
 } // namespace index
 } // namespace clang
 
 #endif

include/clang/Index/IndexDiagnostic.h

+//===--- IndexDiagnostic.h - ------------------------------------*- 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_INDEX_INDEXDIAGNOSTIC_H
+#define LLVM_CLANG_INDEX_INDEXDIAGNOSTIC_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 INDEXSTART
+#include "clang/Basic/DiagnosticIndexKinds.inc"
+#undef DIAG
+  NUM_BUILTIN_INDEX_DIAGNOSTICS
+};
+} // end namespace diag
+} // end namespace clang
+
+#endif // LLVM_CLANG_INDEX_INDEXDIAGNOSTIC_H

include/clang/Index/IndexingAction.h

 //===--- IndexingAction.h - Frontend index action -------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_INDEX_INDEXINGACTION_H
 #define LLVM_CLANG_INDEX_INDEXINGACTION_H
 
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/ArrayRef.h"
 #include <memory>
+#include <string>
 
 namespace clang {
   class ASTContext;
   class ASTReader;
   class ASTUnit;
+  class CompilerInstance;
   class Decl;
   class FrontendAction;
+  class FrontendOptions;
+  class Module;
 
 namespace serialization {
   class ModuleFile;
 }
 
 namespace index {
   class IndexDataConsumer;
+  class IndexUnitWriter;
 
 struct IndexingOptions {
   enum class SystemSymbolFilterKind {
     None,
     DeclarationsOnly,
     All,
   };
 
-  SystemSymbolFilterKind SystemSymbolFilter
-    = SystemSymbolFilterKind::DeclarationsOnly;
+  SystemSymbolFilterKind SystemSymbolFilter =
+      SystemSymbolFilterKind::DeclarationsOnly;
   bool IndexFunctionLocals = false;
 };
 
+struct RecordingOptions {
+  enum class IncludesRecordingKind {
+    None,
+    UserOnly, // only record includes inside non-system files.
+    All,
+  };
+
+  std::string DataDirPath;
+  bool RecordSymbolCodeGenName = false;
+  bool RecordSystemDependencies = true;
+  IncludesRecordingKind RecordIncludes = IncludesRecordingKind::UserOnly;
+};
+
 /// \param WrappedAction another frontend action to wrap over or null.
 std::unique_ptr<FrontendAction>
 createIndexingAction(std::shared_ptr<IndexDataConsumer> DataConsumer,
                      IndexingOptions Opts,
                      std::unique_ptr<FrontendAction> WrappedAction);
 
 void indexASTUnit(ASTUnit &Unit,
                   std::shared_ptr<IndexDataConsumer> DataConsumer,
                   IndexingOptions Opts);
 
 void indexTopLevelDecls(ASTContext &Ctx, ArrayRef<const Decl *> Decls,
                         std::shared_ptr<IndexDataConsumer> DataConsumer,
                         IndexingOptions Opts);
 
 void indexModuleFile(serialization::ModuleFile &Mod, ASTReader &Reader,
                      std::shared_ptr<IndexDataConsumer> DataConsumer,
                      IndexingOptions Opts);
 
+/// \param WrappedAction another frontend action to wrap over or null.
+std::unique_ptr<FrontendAction>
+createIndexDataRecordingAction(const FrontendOptions &FEOpts,
+                               std::unique_ptr<FrontendAction> WrappedAction);
+
 } // namespace index
 } // namespace clang
 
 #endif

include/clang/module.modulemap

module Clang_Diagnostics {
   requires cplusplus
 
   module All { header "Basic/AllDiagnostics.h" export * }
   module Analysis { header "Analysis/AnalysisDiagnostic.h" export * }
   module AST { header "AST/ASTDiagnostic.h" export * }
   module Comment { header "AST/CommentDiagnostic.h" export * }
   module Driver { header "Driver/DriverDiagnostic.h" export * }
   module Frontend { header "Frontend/FrontendDiagnostic.h" export * }
+  module Index { header "Index/IndexDiagnostic.h" export * }
   module Lex { header "Lex/LexDiagnostic.h" export * }
   module Parse { header "Parse/ParseDiagnostic.h" export * }
   module Sema { header "Sema/SemaDiagnostic.h" export * }
   module Serialization { header "Serialization/SerializationDiagnostic.h" export * }
   module Refactoring { header "Tooling/Refactoring/RefactoringDiagnostic.h" export * }
 }
 
 module Clang_Driver {

lib/Basic/DiagnosticIDs.cpp

 VALIDATE_DIAG_SIZE(SERIALIZATION)
 VALIDATE_DIAG_SIZE(LEX)
 VALIDATE_DIAG_SIZE(PARSE)
 VALIDATE_DIAG_SIZE(AST)
 VALIDATE_DIAG_SIZE(COMMENT)
 VALIDATE_DIAG_SIZE(SEMA)
 VALIDATE_DIAG_SIZE(ANALYSIS)
 VALIDATE_DIAG_SIZE(REFACTORING)
+VALIDATE_DIAG_SIZE(INDEX)
 #undef VALIDATE_DIAG_SIZE
 #undef STRINGIFY_NAME
 
 } // namespace anonymous
 
 static const StaticDiagInfoRec StaticDiagInfo[] = {
 #define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR,     \
              SHOWINSYSHEADER, CATEGORY)                                        \

 #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"
 #include "clang/Basic/DiagnosticRefactoringKinds.inc"
+#include "clang/Basic/DiagnosticIndexKinds.inc"
 #undef DIAG
 };
 
 static const unsigned StaticDiagInfoSize = llvm::array_lengthof(StaticDiagInfo);
 
 /// GetDiagInfo - Return the StaticDiagInfoRec entry for the specified DiagID,
 /// or null if the ID is invalid.
 static const StaticDiagInfoRec *GetDiagInfo(unsigned DiagID) {

 CATEGORY(LEX, SERIALIZATION)
 CATEGORY(PARSE, LEX)
 CATEGORY(AST, PARSE)
 CATEGORY(COMMENT, AST)
 CATEGORY(CROSSTU, COMMENT)
 CATEGORY(SEMA, CROSSTU)
 CATEGORY(ANALYSIS, SEMA)
 CATEGORY(REFACTORING, ANALYSIS)
+CATEGORY(INDEX, REFACTORING)
 #undef CATEGORY
 
   // Avoid out of bounds reads.
   if (ID + Offset >= StaticDiagInfoSize)
     return nullptr;
 
   assert(ID < StaticDiagInfoSize && Offset < StaticDiagInfoSize);
 

lib/Driver/Driver.cpp

if (StringRef(TempFile).endswith(".cache")) {
       // In some cases (modules) we'll dump extra data to help with reproducing
       // the crash into a directory next to the output.
       VFS = llvm::sys::path::filename(TempFile);
       llvm::sys::path::append(VFS, "vfs", "vfs.yaml");
     }
   }
 
   // Assume associated files are based off of the first temporary file.
-  CrashReportInfo CrashInfo(TempFiles[0], VFS);
+  CrashReportInfo CrashInfo(
+      TempFiles[0], VFS,
+      C.getArgs().getLastArgValue(options::OPT_index_store_path));
 
   std::string Script = CrashInfo.Filename.rsplit('.').first.str() + ".sh";
   std::error_code EC;
   llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::F_Excl);
   if (EC) {
     Diag(clang::diag::note_drv_command_failed_diag_msg)
         << "Error generating run script: " + Script + " " + EC.message();
   } else {

lib/Driver/Job.cpp

IsInclude = llvm::StringSwitch<bool>(Flag)
     .Cases("-idirafter", "-internal-isystem", "-iwithprefix", true)
     .Cases("-internal-externc-isystem", "-iprefix", true)
     .Cases("-iwithprefixbefore", "-isystem", "-iquote", true)
     .Cases("-isysroot", "-I", "-F", "-resource-dir", true)
     .Cases("-iframework", "-include-pch", true)
     .Default(false);
   if (IsInclude)
     return HaveCrashVFS ? false : true;
+  if (StringRef(Flag).startswith("-index-store-path"))
+    return true;
 
   // The remaining flags are treated as a single argument.
 
   // These flags are all of the form -Flag and have no second argument.
   ShouldSkip = llvm::StringSwitch<bool>(Flag)
     .Cases("-M", "-MM", "-MG", "-MP", "-MD", true)
     .Case("-MMD", true)
     .Default(false);
rewriteIncludes(const llvm::ArrayRef<const char *> &Args, size_t Idx,
   assert(NumArgs == 2 && "Not expecting more than two arguments");
   StringRef Inc(Args[Idx + NumArgs - 1]);
   if (!getAbsPath(Inc, NewInc))
     return;
   IncFlags.push_back(SmallString<128>(Args[Idx]));
   IncFlags.push_back(std::move(NewInc));
 }
 
 void Command::Print(raw_ostream &OS, const char *Terminator, bool Quote,
                     CrashReportInfo *CrashInfo) const {
   // Always quote the exe.
   OS << ' ';
   printArg(OS, Executable, /*Quote=*/true);
 
   llvm::ArrayRef<const char *> Args = Arguments;
   llvm::SmallVector<const char *, 128> ArgsRespFile;
   if (ResponseFile != nullptr) {
     buildArgvForResponseFile(ArgsRespFile);
     Args = ArrayRef<const char *>(ArgsRespFile).slice(1); // no executable name
   }
 
   bool HaveCrashVFS = CrashInfo && !CrashInfo->VFSPath.empty();
+  bool HaveIndexStorePath = CrashInfo && !CrashInfo->IndexStorePath.empty();
   for (size_t i = 0, e = Args.size(); i < e; ++i) {
     const char *const Arg = Args[i];
 
     if (CrashInfo) {
       int NumArgs = 0;
       bool IsInclude = false;
       if (skipArgs(Arg, HaveCrashVFS, NumArgs, IsInclude)) {
         i += NumArgs - 1;
if (CrashInfo && HaveCrashVFS) {
 
     std::string ModCachePath = "-fmodules-cache-path=";
     ModCachePath.append(RelModCacheDir.c_str());
 
     OS << ' ';
     printArg(OS, ModCachePath, Quote);
   }
 
+  if (CrashInfo && HaveIndexStorePath) {
+    SmallString<128> IndexStoreDir;
+
+    if (HaveCrashVFS) {
+      IndexStoreDir = llvm::sys::path::parent_path(
+          llvm::sys::path::parent_path(CrashInfo->VFSPath));
+      llvm::sys::path::append(IndexStoreDir, "index-store");
+    } else {
+      IndexStoreDir = "index-store";
+    }
+
+    OS << ' ';
+    printArg(OS, "-index-store-path", Quote);
+    OS << ' ';
+    printArg(OS, IndexStoreDir.c_str(), Quote);
+  }
+
   if (ResponseFile != nullptr) {
     OS << "\n Arguments passed via response file:\n";
     writeResponseFile(OS);
     // Avoiding duplicated newline terminator, since FileLists are
     // newline-separated.
     if (Creator.getResponseFilesSupport() != Tool::RF_FileList)
       OS << "\n";
     OS << " (end of response file)";

lib/Driver/ToolChains/Clang.cpp

#endif
   // Pass the path to compiler resource files.
   CmdArgs.push_back("-resource-dir");
   CmdArgs.push_back(D.ResourceDir.c_str());
 
   Args.AddLastArg(CmdArgs, options::OPT_working_directory);
 
   RenderARCMigrateToolOptions(D, Args, CmdArgs);
 
+  if (Args.hasArg(options::OPT_index_store_path)) {
+    Args.AddLastArg(CmdArgs, options::OPT_index_store_path);
+    Args.AddLastArg(CmdArgs, options::OPT_index_ignore_system_symbols);
+    Args.AddLastArg(CmdArgs, options::OPT_index_record_codegen_name);
+
+    // If '-o' is passed along with '-fsyntax-only' pass it along the cc1
+    // invocation so that the index action knows what the out file is.
+    if (isa<CompileJobAction>(JA) && JA.getType() == types::TY_Nothing) {
+      Args.AddLastArg(CmdArgs, options::OPT_o);
+    }
+  }
+
   // Add preprocessing options like -I, -D, etc. if we are using the
   // preprocessor.
   //
   // FIXME: Support -fpreprocessed
   if (types::getPreprocessedType(InputType) != types::TY_INVALID)
     AddPreprocessingOptions(C, JA, D, Args, CmdArgs, Output, Inputs);
 
   // Don't warn about "clang -c -DPIC -fPIC test.i" because libtool.m4 assumes
   // that "The compiler can only warn and ignore the option if not recognized".

lib/Driver/ToolChains/Darwin.cpp

void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   // -filelist linker option.
   llvm::opt::ArgStringList InputFileList;
 
   // The logic here is derived from gcc's behavior; most of which
   // comes from specs (starting with link_command). Consult gcc for
   // more information.
   ArgStringList CmdArgs;
 
+  Args.ClaimAllArgs(options::OPT_index_store_path);
+  Args.ClaimAllArgs(options::OPT_index_ignore_system_symbols);
+  Args.ClaimAllArgs(options::OPT_index_record_codegen_name);
+
   /// Hack(tm) to ignore linking errors when we are doing ARC migration.
   if (Args.hasArg(options::OPT_ccc_arcmt_check,
                   options::OPT_ccc_arcmt_migrate)) {
     for (const auto &Arg : Args)
       Arg->claim();
     const char *Exec =
         Args.MakeArgString(getToolChain().GetProgramPath("touch"));
     CmdArgs.push_back(Output.getFilename());

lib/Frontend/CompilerInstance.cpp

 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/LogDiagnosticPrinter.h"
 #include "clang/Frontend/SerializedDiagnosticPrinter.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Frontend/Utils.h"
 #include "clang/Frontend/VerifyDiagnosticConsumer.h"
+#include "clang/Index/IndexingAction.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/PTHManager.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/GlobalModuleIndex.h"
compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
   Instance.setFileManager(&ImportingInstance.getFileManager());
   Instance.createSourceManager(Instance.getFileManager());
   SourceManager &SourceMgr = Instance.getSourceManager();
   SourceMgr.setModuleBuildStack(
     ImportingInstance.getSourceManager().getModuleBuildStack());
   SourceMgr.pushModuleBuildStack(ModuleName,
     FullSourceLoc(ImportLoc, ImportingInstance.getSourceManager()));
 
+  // Pass along the GenModuleActionWrapper callback
+  auto WrapGenModuleAction = ImportingInstance.getGenModuleActionWrapper();
+  Instance.setGenModuleActionWrapper(WrapGenModuleAction);
+
   // If we're collecting module dependencies, we need to share a collector
   // between all of the module CompilerInstances. Other than that, we don't
   // want to produce any dependency output from the module build.
   Instance.setModuleDepCollector(ImportingInstance.getModuleDepCollector());
   Inv.getDependencyOutputOpts() = DependencyOutputOptions();
 
   ImportingInstance.getDiagnostics().Report(ImportLoc,
                                             diag::remark_module_build)
     << ModuleName << ModuleFileName;
 
   PreBuildStep(Instance);
 
   // Execute the action to actually build the module in-place. Use a separate
   // thread so that we get a stack large enough.
   const unsigned ThreadStackSize = 8 << 20;
   llvm::CrashRecoveryContext CRC;
   CRC.RunSafelyOnThread(
       [&]() {
-        GenerateModuleFromModuleMapAction Action;
-        Instance.ExecuteAction(Action);
+        std::unique_ptr<FrontendAction> Action(
+            new GenerateModuleFromModuleMapAction);
+        if (WrapGenModuleAction)
+          Action = WrapGenModuleAction(FrontendOpts, std::move(Action));
+        Instance.ExecuteAction(*Action);
       },
       ThreadStackSize);
 
   PostBuildStep(Instance);
 
   ImportingInstance.getDiagnostics().Report(ImportLoc,
                                             diag::remark_module_build_done)
     << ModuleName;

lib/Frontend/CompilerInvocation.cpp

static InputKind ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
   Opts.ObjCMTWhiteListPath = Args.getLastArgValue(OPT_objcmt_whitelist_dir_path);
 
   if (Opts.ARCMTAction != FrontendOptions::ARCMT_None &&
       Opts.ObjCMTAction != FrontendOptions::ObjCMT_None) {
     Diags.Report(diag::err_drv_argument_not_allowed_with)
       << "ARC migration" << "ObjC migration";
   }
 
+  Opts.IndexStorePath = Args.getLastArgValue(OPT_index_store_path);
+  Opts.IndexIgnoreSystemSymbols = Args.hasArg(OPT_index_ignore_system_symbols);
+  Opts.IndexRecordCodegenName = Args.hasArg(OPT_index_record_codegen_name);
+
   InputKind DashX(InputKind::Unknown);
   if (const Arg *A = Args.getLastArg(OPT_x)) {
     StringRef XValue = A->getValue();
 
     // Parse suffixes: '<lang>(-header|[-module-map][-cpp-output])'.
     // FIXME: Supporting '<lang>-header-cpp-output' would be useful.
     bool Preprocessed = XValue.consume_back("-cpp-output");
     bool ModuleMap = XValue.consume_back("-module-map");

lib/FrontendTool/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Option
   Support
   )
 
 set(link_libs
   clangBasic
   clangCodeGen
   clangDriver
   clangFrontend
+  clangIndex
   clangRewriteFrontend
   )
 
 if(CLANG_ENABLE_ARCMT)
   list(APPEND link_libs
     clangARCMigrate
     )
 endif()

lib/FrontendTool/ExecuteCompilerInvocation.cpp

 #include "clang/Config/config.h"
 #include "clang/Driver/Options.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/Frontend/Utils.h"
+#include "clang/Index/IndexingAction.h"
 #include "clang/Rewrite/Frontend/FrontendActions.h"
 #include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/ErrorHandling.h"
 using namespace clang;
 using namespace llvm::opt;
if (CI.getFrontendOpts().ProgramAction != frontend::MigrateSource &&
     if (FEOpts.ObjCMTAction != FrontendOptions::ObjCMT_None) {
       Act = llvm::make_unique<arcmt::ObjCMigrateAction>(std::move(Act),
                                                         FEOpts.MTMigrateDir,
                                                         FEOpts.ObjCMTAction);
     }
   }
 #endif
 
+  if (!FEOpts.IndexStorePath.empty()) {
+    Act = index::createIndexDataRecordingAction(FEOpts, std::move(Act));
+    // Also wrap any GenerateModuleActions created while loading modules
+    CI.setGenModuleActionWrapper(&index::createIndexDataRecordingAction);
+  }
+
   // If there are any AST files to merge, create a frontend action
   // adaptor to perform the merge.
   if (!FEOpts.ASTMergeFiles.empty())
     Act = llvm::make_unique<ASTMergeAction>(std::move(Act),
                                             FEOpts.ASTMergeFiles);
 
   return Act;
 }

lib/Index/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Core
   Support
   )
 
 add_clang_library(clangIndex
   CodegenNameGenerator.cpp
   CommentToXML.cpp
+  FileIndexRecord.cpp
   IndexBody.cpp
   IndexDecl.cpp
   IndexingAction.cpp
   IndexingContext.cpp
   IndexSymbol.cpp
   IndexTypeSourceInfo.cpp
   USRGeneration.cpp
 
   ADDITIONAL_HEADERS
   IndexingContext.h
   SimpleFormatContext.h
 
   LINK_LIBS
   clangAST
   clangBasic
   clangFormat
   clangFrontend
+  clangLex
   clangRewrite
   clangSerialization
   clangToolingCore
   )

lib/Index/FileIndexRecord.h

+//===--- FileIndexRecord.h - Index data per file --------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_INDEX_FILEINDEXRECORD_H
+#define LLVM_CLANG_LIB_INDEX_FILEINDEXRECORD_H
+
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+#include <vector>
+
+namespace clang {
+class IdentifierInfo;
+
+namespace index {
+
+/// Stores the declaration occurrences seen in a particular source or header
+/// file of a translation unit
+class FileIndexRecord {
+public:
+  struct DeclOccurrence {
+    SymbolRoleSet Roles;
+    unsigned Offset;
+    const Decl *Dcl;
+    SmallVector<SymbolRelation, 3> Relations;
+
+    DeclOccurrence(SymbolRoleSet R, unsigned Offset, const Decl *D,
+                   ArrayRef<SymbolRelation> Relations)
+        : Roles(R), Offset(Offset), Dcl(D),
+          Relations(Relations.begin(), Relations.end()) {}
+
+    friend bool operator<(const DeclOccurrence &LHS,
+                          const DeclOccurrence &RHS) {
+      return LHS.Offset < RHS.Offset;
+    }
+  };
+
+private:
+  FileID FID;
+  bool IsSystem;
+  std::vector<DeclOccurrence> Decls;
+
+public:
+  FileIndexRecord(FileID FID, bool isSystem) : FID(FID), IsSystem(isSystem) {}
+
+  ArrayRef<DeclOccurrence> getDeclOccurrences() const { return Decls; }
+
+  FileID getFileID() const { return FID; }
+  bool isSystem() const { return IsSystem; }
+
+  /// Adds an occurrence of the canonical declaration \c D at the supplied
+  /// \c Offset
+  ///
+  /// \param Roles the roles the occurrence fulfills in this position.
+  /// \param Offset the offset in the file of this occurrence.
+  /// \param D the canonical declaration this is an occurrence of.
+  /// \param Relations the set of symbols related to this occurrence.
+  void addDeclOccurence(SymbolRoleSet Roles, unsigned Offset, const Decl *D,
+                        ArrayRef<SymbolRelation> Relations);
+  void sortOccurrencesByOffset();
+  void print(llvm::raw_ostream &OS) const;
+};
+
+} // end namespace index
+} // end namespace clang
+
+#endif

lib/Index/FileIndexRecord.cpp

+//===--- FileIndexRecord.cpp - Index data per file ------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "FileIndexRecord.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclTemplate.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Path.h"
+
+using namespace clang;
+using namespace clang::index;
+
+void FileIndexRecord::addDeclOccurence(SymbolRoleSet Roles, unsigned Offset,
+                                       const Decl *D,
+                                       ArrayRef<SymbolRelation> Relations) {
+  assert(D->isCanonicalDecl() &&
+         "Occurrences should be associated with their canonical decl");
+
+  Decls.emplace_back(Roles, Offset, D, Relations);
+}
+
+void FileIndexRecord::sortOccurrencesByOffset() {
+  std::sort(Decls.begin(), Decls.end());
+}
+
+void FileIndexRecord::print(llvm::raw_ostream &OS) const {
+  OS << "DECLS BEGIN ---\n";
+  for (auto &DclInfo : Decls) {
+    auto D = DclInfo.Dcl;
+    SourceManager &SM = D->getASTContext().getSourceManager();
+    SourceLocation Loc = SM.getFileLoc(D->getLocation());
+    PresumedLoc PLoc = SM.getPresumedLoc(Loc);
+    OS << llvm::sys::path::filename(PLoc.getFilename()) << ':' << PLoc.getLine()
+       << ':' << PLoc.getColumn();
+
+    if (auto ND = dyn_cast<NamedDecl>(D)) {
+      OS << ' ' << ND->getNameAsString();
+    }
+
+    OS << '\n';
+  }
+  OS << "DECLS END ---\n";
+}

lib/Index/IndexingAction.cpp

 //===- IndexingAction.cpp - Frontend index action -------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Index/IndexingAction.h"
-#include "clang/Index/IndexDataConsumer.h"
+#include "FileIndexRecord.h"
 #include "IndexingContext.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/MultiplexConsumer.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexDiagnostic.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Serialization/ASTReader.h"
 
 using namespace clang;
 using namespace clang::index;
 
 void IndexDataConsumer::_anchor() {}
 
 bool IndexDataConsumer::handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
                                             ArrayRef<SymbolRelation> Relations,
                                             FileID FID, unsigned Offset,
+                                            bool IsInSystemFile,
                                             ASTNodeInfo ASTNode) {
   return true;
 }
 
 bool IndexDataConsumer::handleMacroOccurence(const IdentifierInfo *Name,
-                                             const MacroInfo *MI, SymbolRoleSet Roles,
-                                             FileID FID, unsigned Offset) {
+                                             const MacroInfo *MI,
+                                             SymbolRoleSet Roles, FileID FID,
+                                             unsigned Offset,
+                                             bool IsInSystemFile) {
   return true;
 }
 
 bool IndexDataConsumer::handleModuleOccurence(const ImportDecl *ImportD,
-                                              SymbolRoleSet Roles,
-                                              FileID FID, unsigned Offset) {
+                                              SymbolRoleSet Roles, FileID FID,
+                                              unsigned Offset,
+                                              bool IsInSystemFile) {
   return true;
 }
 
 namespace {
 
 class IndexASTConsumer : public ASTConsumer {
   IndexingContext &IndexCtx;
 
protected:
   void HandleTranslationUnit(ASTContext &Ctx) override {
   }
 };
 
 class IndexActionBase {
 protected:
   std::shared_ptr<IndexDataConsumer> DataConsumer;
   IndexingContext IndexCtx;
+  IsSystemFileCache IsSystemCache;
 
   IndexActionBase(std::shared_ptr<IndexDataConsumer> dataConsumer,
                   IndexingOptions Opts)
       : DataConsumer(std::move(dataConsumer)),
-      IndexCtx(Opts, *DataConsumer) {}
+        IndexCtx(Opts, *DataConsumer, IsSystemCache) {}
 
-  std::unique_ptr<IndexASTConsumer> createIndexASTConsumer() {
+  std::unique_ptr<IndexASTConsumer>
+  createIndexASTConsumer(CompilerInstance &CI) {
+    IsSystemCache.setSysrootPath(CI.getHeaderSearchOpts().Sysroot);
     return llvm::make_unique<IndexASTConsumer>(IndexCtx);
   }
 
   void finish() {
     DataConsumer->finish();
   }
 };
 
 class IndexAction : public ASTFrontendAction, IndexActionBase {
 public:
   IndexAction(std::shared_ptr<IndexDataConsumer> DataConsumer,
               IndexingOptions Opts)
     : IndexActionBase(std::move(DataConsumer), Opts) {}
 
 protected:
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
                                                  StringRef InFile) override {
-    return createIndexASTConsumer();
+    return createIndexASTConsumer(CI);
   }
 
   void EndSourceFileAction() override {
     FrontendAction::EndSourceFileAction();
     finish();
   }
 };
 
 class WrappingIndexAction : public WrapperFrontendAction, IndexActionBase {
-  bool IndexActionFailed = false;
+  bool CreatedASTConsumer = false;
 
 public:
   WrappingIndexAction(std::unique_ptr<FrontendAction> WrappedAction,
                       std::shared_ptr<IndexDataConsumer> DataConsumer,
                       IndexingOptions Opts)
     : WrapperFrontendAction(std::move(WrappedAction)),
       IndexActionBase(std::move(DataConsumer), Opts) {}
 
 protected:
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
                                                  StringRef InFile) override;
   void EndSourceFileAction() override;
 };
 
 } // anonymous namespace
 
 void WrappingIndexAction::EndSourceFileAction() {
   // Invoke wrapped action's method.
   WrapperFrontendAction::EndSourceFileAction();
-  if (!IndexActionFailed)
+  if (CreatedASTConsumer)
     finish();
 }
 
 std::unique_ptr<ASTConsumer>
 WrappingIndexAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
   auto OtherConsumer = WrapperFrontendAction::CreateASTConsumer(CI, InFile);
-  if (!OtherConsumer) {
-    IndexActionFailed = true;
+  if (!OtherConsumer)
     return nullptr;
-  }
 
+  CreatedASTConsumer = true;
   std::vector<std::unique_ptr<ASTConsumer>> Consumers;
   Consumers.push_back(std::move(OtherConsumer));
-  Consumers.push_back(createIndexASTConsumer());
+  Consumers.push_back(createIndexASTConsumer(CI));
   return llvm::make_unique<MultiplexConsumer>(std::move(Consumers));
 }
 
 std::unique_ptr<FrontendAction>
 index::createIndexingAction(std::shared_ptr<IndexDataConsumer> DataConsumer,
                             IndexingOptions Opts,
                             std::unique_ptr<FrontendAction> WrappedAction) {
   if (WrappedAction)
     return llvm::make_unique<WrappingIndexAction>(std::move(WrappedAction),
                                                   std::move(DataConsumer),
                                                   Opts);
   return llvm::make_unique<IndexAction>(std::move(DataConsumer), Opts);
 }
 
 
 static bool topLevelDeclVisitor(void *context, const Decl *D) {
   IndexingContext &IndexCtx = *static_cast<IndexingContext*>(context);
   return IndexCtx.indexTopLevelDecl(D);
 }
 
 static void indexTranslationUnit(ASTUnit &Unit, IndexingContext &IndexCtx) {
   Unit.visitLocalTopLevelDecls(&IndexCtx, topLevelDeclVisitor);
 }
 
 void index::indexASTUnit(ASTUnit &Unit,
                          std::shared_ptr<IndexDataConsumer> DataConsumer,
                          IndexingOptions Opts) {
-  IndexingContext IndexCtx(Opts, *DataConsumer);
+  IsSystemFileCache IsSystemCache;
+  IndexingContext IndexCtx(Opts, *DataConsumer, IsSystemCache);
   IndexCtx.setASTContext(Unit.getASTContext());
   DataConsumer->initialize(Unit.getASTContext());
   indexTranslationUnit(Unit, IndexCtx);
   DataConsumer->finish();
 }
 
 void index::indexTopLevelDecls(ASTContext &Ctx, ArrayRef<const Decl *> Decls,
                                std::shared_ptr<IndexDataConsumer> DataConsumer,
                                IndexingOptions Opts) {
-  IndexingContext IndexCtx(Opts, *DataConsumer);
+  IsSystemFileCache IsSystemCache;
+  IndexingContext IndexCtx(Opts, *DataConsumer, IsSystemCache);
   IndexCtx.setASTContext(Ctx);
 
   DataConsumer->initialize(Ctx);
   for (const Decl *D : Decls)
     IndexCtx.indexTopLevelDecl(D);
   DataConsumer->finish();
 }
 
 void index::indexModuleFile(serialization::ModuleFile &Mod,
                             ASTReader &Reader,
                             std::shared_ptr<IndexDataConsumer> DataConsumer,
                             IndexingOptions Opts) {
   ASTContext &Ctx = Reader.getContext();
-  IndexingContext IndexCtx(Opts, *DataConsumer);
+  IsSystemFileCache IsSystemCache;
+  IndexingContext IndexCtx(Opts, *DataConsumer, IsSystemCache);
   IndexCtx.setASTContext(Ctx);
   DataConsumer->initialize(Ctx);
 
   for (const Decl *D :Reader.getModuleFileLevelDecls(Mod)) {
     IndexCtx.indexTopLevelDecl(D);
   }
   DataConsumer->finish();
 }
+
+//===----------------------------------------------------------------------===//
+// Index Data Recording
+//===----------------------------------------------------------------------===//
+
+namespace {
+
+class IndexDataRecorder : public IndexDataConsumer {
+  Preprocessor *PP = nullptr;
+  typedef llvm::DenseMap<FileID, std::unique_ptr<FileIndexRecord>>
+      RecordByFileTy;
+  RecordByFileTy RecordByFile;
+
+public:
+  void init(Preprocessor &PreProc, ASTContext &Ctx) {
+    PP = &PreProc;
+    initialize(Ctx);
+  }
+
+  RecordByFileTy::const_iterator record_begin() const {
+    return RecordByFile.begin();
+  }
+
+  RecordByFileTy::const_iterator record_end() const {
+    return RecordByFile.end();
+  }
+
+  bool record_empty() const { return RecordByFile.empty(); }
+
+  void finish() override {
+    // Sort occurrences so ordering doesn't impact the hashing
+    for (auto &Entry : RecordByFile)
+      Entry.getSecond()->sortOccurrencesByOffset();
+  }
+
+private:
+  bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
+                           ArrayRef<SymbolRelation> Relations, FileID FID,
+                           unsigned Offset, bool IsInSystemFile,
+                           ASTNodeInfo ASTNode) override {
+    // Ignore the predefines buffer.
+    if (FID == PP->getPredefinesFileID())
+      return true;
+
+    FileIndexRecord &Rec = getFileIndexRecord(FID, IsInSystemFile);
+    Rec.addDeclOccurence(Roles, Offset, D, Relations);
+    return true;
+  }
+
+  FileIndexRecord &getFileIndexRecord(FileID FID, bool IsInSystemFile) {
+    auto &Entry = RecordByFile[FID];
+    if (!Entry) {
+      Entry.reset(new FileIndexRecord(FID, IsInSystemFile));
+    }
+    return *Entry;
+  }
+};
+
+struct IncludeLocation {
+  const FileEntry *Source;
+  const FileEntry *Target;
+  unsigned Line;
+};
+
+class IncludePPCallbacks : public PPCallbacks {
+  IsSystemFileCache &IsSystemCache;
+  RecordingOptions::IncludesRecordingKind RecordIncludes;
+  std::vector<IncludeLocation> &Includes;
+  SourceManager &SourceMgr;
+
+public:
+  IncludePPCallbacks(IsSystemFileCache &IsSystemCache,
+                     RecordingOptions::IncludesRecordingKind RecordIncludes,
+                     std::vector<IncludeLocation> &IncludesForFile,
+                     SourceManager &SourceMgr)
+      : IsSystemCache(IsSystemCache), RecordIncludes(RecordIncludes),
+        Includes(IncludesForFile), SourceMgr(SourceMgr) {}
+
+private:
+  void addInclude(SourceLocation From, const FileEntry *To) {
+    assert(To);
+    if (RecordIncludes == RecordingOptions::IncludesRecordingKind::None)
+      return;
+
+    std::pair<FileID, unsigned> LocInfo =
+        SourceMgr.getDecomposedExpansionLoc(From);
+
+    if (LocInfo.first.isInvalid())
+      return; // Ignore invalid locations
+
+    switch (RecordIncludes) {
+    case RecordingOptions::IncludesRecordingKind::None:
+      llvm_unreachable("should have already checked in the beginning");
+    case RecordingOptions::IncludesRecordingKind::UserOnly:
+      if (IsSystemCache.isSystem(LocInfo.first, SourceMgr))
+        return; // Ignore includes of system headers.
+      break;
+    case RecordingOptions::IncludesRecordingKind::All:
+      break;
+    }
+
+    if (auto *FE = SourceMgr.getFileEntryForID(LocInfo.first)) {
+      auto lineNo = SourceMgr.getLineNumber(LocInfo.first, LocInfo.second);
+      Includes.push_back({FE, To, lineNo});
+    }
+  }
+
+  virtual void InclusionDirective(SourceLocation HashLoc,
+                                  const Token &IncludeTok, StringRef FileName,
+                                  bool IsAngled, CharSourceRange FilenameRange,
+                                  const FileEntry *File, StringRef SearchPath,
+                                  StringRef RelativePath,
+                                  const Module *Imported) override {
+    if (HashLoc.isFileID() && File && File->isValid())
+      addInclude(HashLoc, File);
+  }
+};
+
+/// Abstract interface for providing the file and module dependencies of a
+/// translation unit, as well as the set of file to file inclusions
+class IndexDependencyProvider {
+public:
+  virtual ~IndexDependencyProvider() {}
+
+  virtual void visitFileDependencies(
+      const CompilerInstance &CI,
+      llvm::function_ref<void(const FileEntry *FE, bool isSystem)> visitor) = 0;
+  virtual void
+  visitIncludes(llvm::function_ref<void(const FileEntry *Source, unsigned Line,
+                                        const FileEntry *Target)>
+                    visitor) = 0;
+  virtual void visitModuleImports(
+      const CompilerInstance &CI,
+      llvm::function_ref<void(serialization::ModuleFile &Mod, bool isSystem)>
+          visitor) = 0;
+};
+
+/// Collects and provides the file and module dependency information, including
+/// file to file inclusions, for the source files in a translation unit
+class SourceFilesIndexDependencyCollector : public DependencyCollector,
+                                            public IndexDependencyProvider {
+  IsSystemFileCache &IsSystemCache;
+  RecordingOptions RecordOpts;
+  llvm::SetVector<const FileEntry *> Entries;
+  llvm::BitVector IsSystemByUID;
+  std::vector<IncludeLocation> Includes;
+  SourceManager *SourceMgr = nullptr;
+
+public:
+  SourceFilesIndexDependencyCollector(IsSystemFileCache &SysrootPath,
+                                      RecordingOptions recordOpts)
+      : IsSystemCache(SysrootPath), RecordOpts(recordOpts) {}
+
+  void attachToPreprocessor(Preprocessor &PP) override {
+    DependencyCollector::attachToPreprocessor(PP);
+    PP.addPPCallbacks(llvm::make_unique<IncludePPCallbacks>(
+        IsSystemCache, RecordOpts.RecordIncludes, Includes,
+        PP.getSourceManager()));
+  }
+
+  void setSourceManager(SourceManager *SourceMgr) {
+    this->SourceMgr = SourceMgr;
+  }
+
+  void visitFileDependencies(
+      const CompilerInstance &CI,
+      llvm::function_ref<void(const FileEntry *FE, bool isSystem)> visitor)
+      override {
+    for (auto *FE : getEntries()) {
+      visitor(FE, isSystemFile(FE));
+    }
+  }
+
+  void
+  visitIncludes(llvm::function_ref<void(const FileEntry *Source, unsigned Line,
+                                        const FileEntry *Target)>
+                    visitor) override {
+    for (auto &Include : Includes)
+      visitor(Include.Source, Include.Line, Include.Target);
+  }
+
+  void visitModuleImports(
+      const CompilerInstance &CI,
+      llvm::function_ref<void(serialization::ModuleFile &Mod, bool isSystem)>
+          visitor) override {
+    HeaderSearch &HS = CI.getPreprocessor().getHeaderSearchInfo();
+
+    if (auto Reader = CI.getModuleManager()) {
+      Reader->getModuleManager().visit(
+          [&](serialization::ModuleFile &Mod) -> bool {
+            bool isSystemMod = false;
+            if (Mod.isModule()) {
+              if (auto *M =
+                      HS.lookupModule(Mod.ModuleName, /*AllowSearch=*/false))
+                isSystemMod = M->IsSystem;
+            }
+            if (!isSystemMod || needSystemDependencies())
+              visitor(Mod, isSystemMod);
+            return true; // skip module dependencies.
+          });
+    }
+  }
+
+private:
+  bool isSystemFile(const FileEntry *FE) {
+    auto UID = FE->getUID();
+    return IsSystemByUID.size() > UID && IsSystemByUID[UID];
+  }
+
+  ArrayRef<const FileEntry *> getEntries() const {
+    return Entries.getArrayRef();
+  }
+
+  bool needSystemDependencies() override {
+    return RecordOpts.RecordSystemDependencies;
+  }
+
+  bool sawDependency(StringRef Filename, bool FromModule, bool IsSystem,
+                     bool IsModuleFile, bool IsMissing) override {
+    bool sawIt = DependencyCollector::sawDependency(
+        Filename, FromModule, IsSystem, IsModuleFile, IsMissing);
+    if (auto *FE = SourceMgr->getFileManager().getFile(Filename)) {
+      if (sawIt)
+        Entries.insert(FE);
+      // Record system-ness for all files that we pass through.
+      if (IsSystemByUID.size() < FE->getUID() + 1)
+        IsSystemByUID.resize(FE->getUID() + 1);
+      IsSystemByUID[FE->getUID()] = IsSystem || isInSysroot(Filename);
+    }
+    return sawIt;
+  }
+
+  bool isInSysroot(StringRef Filename) {
+    StringRef SysrootPath = IsSystemCache.getSysrootPath();
+    return !SysrootPath.empty() && Filename.startswith(SysrootPath);
+  }
+};
+
+class IndexRecordActionBase {
+protected:
+  RecordingOptions RecordOpts;
+  IndexDataRecorder Recorder;
+  IndexingContext IndexCtx;
+  IsSystemFileCache IsSystemCache;
+  SourceFilesIndexDependencyCollector DepCollector;
+
+  IndexRecordActionBase(IndexingOptions IndexOpts, RecordingOptions recordOpts)
+      : RecordOpts(std::move(recordOpts)),
+        IndexCtx(IndexOpts, Recorder, IsSystemCache),
+        DepCollector(IsSystemCache, RecordOpts) {}
+
+  std::unique_ptr<IndexASTConsumer>
+  createIndexASTConsumer(CompilerInstance &CI) {
+    IsSystemCache.setSysrootPath(CI.getHeaderSearchOpts().Sysroot);
+
+    Preprocessor &PP = CI.getPreprocessor();
+    Recorder.init(PP, CI.getASTContext());
+
+    DepCollector.setSourceManager(&CI.getSourceManager());
+    DepCollector.attachToPreprocessor(PP);
+
+    return llvm::make_unique<IndexASTConsumer>(IndexCtx);
+  }
+
+  void finish(CompilerInstance &CI);
+};
+
+class IndexRecordAction : public ASTFrontendAction, IndexRecordActionBase {
+public:
+  IndexRecordAction(IndexingOptions IndexOpts, RecordingOptions RecordOpts)
+      : IndexRecordActionBase(std::move(IndexOpts), std::move(RecordOpts)) {}
+
+protected:
+  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+                                                 StringRef InFile) override {
+    return createIndexASTConsumer(CI);
+  }
+
+  void EndSourceFileAction() override {
+    FrontendAction::EndSourceFileAction();
+    finish(getCompilerInstance());
+  }
+};
+
+class WrappingIndexRecordAction : public WrapperFrontendAction,
+                                  IndexRecordActionBase {
+  bool CreatedASTConsumer = false;
+
+public:
+  WrappingIndexRecordAction(std::unique_ptr<FrontendAction> WrappedAction,
+                            IndexingOptions IndexOpts,
+                            RecordingOptions RecordOpts)
+      : WrapperFrontendAction(std::move(WrappedAction)),
+        IndexRecordActionBase(std::move(IndexOpts), std::move(RecordOpts)) {}
+
+protected:
+  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+                                                 StringRef InFile) override {
+    auto OtherConsumer = WrapperFrontendAction::CreateASTConsumer(CI, InFile);
+    if (!OtherConsumer)
+      return nullptr;
+
+    CreatedASTConsumer = true;
+    std::vector<std::unique_ptr<ASTConsumer>> Consumers;
+    Consumers.push_back(std::move(OtherConsumer));
+    Consumers.push_back(createIndexASTConsumer(CI));
+    return llvm::make_unique<MultiplexConsumer>(std::move(Consumers));
+  }
+
+  void EndSourceFileAction() override {
+    // Invoke wrapped action's method.
+    WrapperFrontendAction::EndSourceFileAction();
+    if (CreatedASTConsumer)
+      finish(getCompilerInstance());
+  }
+};
+
+} // anonymous namespace
+
+static void writeUnitData(const CompilerInstance &CI,
+                          IndexDataRecorder &Recorder,
+                          IndexDependencyProvider &DepProvider,
+                          IndexingOptions IndexOpts,
+                          RecordingOptions RecordOpts, StringRef OutputFile,
+                          const FileEntry *RootFile, Module *UnitModule,
+                          StringRef SysrootPath);
+
+void IndexRecordActionBase::finish(CompilerInstance &CI) {
+  // We may emit more diagnostics so do the begin/end source file invocations
+  // on the diagnostic client.
+  // FIXME: FrontendAction::EndSourceFile() should probably not call
+  // CI.getDiagnosticClient().EndSourceFile()' until after it has called
+  // 'EndSourceFileAction()', so that code executing during
+  // EndSourceFileAction() can emit diagnostics. If this is fixed,
+  // DiagClientBeginEndRAII can go away.
+  struct DiagClientBeginEndRAII {
+    CompilerInstance &CI;
+    DiagClientBeginEndRAII(CompilerInstance &CI) : CI(CI) {
+      CI.getDiagnosticClient().BeginSourceFile(CI.getLangOpts());
+    }
+    ~DiagClientBeginEndRAII() { CI.getDiagnosticClient().EndSourceFile(); }
+  } diagClientBeginEndRAII(CI);
+
+  SourceManager &SM = CI.getSourceManager();
+  HeaderSearch &HS = CI.getPreprocessor().getHeaderSearchInfo();
+
+  std::string OutputFile = CI.getFrontendOpts().OutputFile;
+  if (OutputFile.empty()) {
+    OutputFile = CI.getFrontendOpts().Inputs[0].getFile();
+    OutputFile += ".o";
+  }
+
+  const FileEntry *RootFile = nullptr;
+  Module *UnitMod = nullptr;
+  bool isModuleGeneration = CI.getLangOpts().isCompilingModule();
+  if (!isModuleGeneration &&
+      CI.getFrontendOpts().ProgramAction != frontend::GeneratePCH) {
+    RootFile = SM.getFileEntryForID(SM.getMainFileID());
+  }
+  if (isModuleGeneration) {
+    UnitMod = HS.lookupModule(CI.getLangOpts().CurrentModule,
+                              /*AllowSearch=*/false);
+  }
+  Recorder.finish();
+  writeUnitData(CI, Recorder, DepCollector, IndexCtx.getIndexOpts(), RecordOpts,
+                OutputFile, RootFile, UnitMod, IsSystemCache.getSysrootPath());
+}
+
+static void writeUnitData(const CompilerInstance &CI,
+                          IndexDataRecorder &Recorder,
+                          IndexDependencyProvider &DepProvider,
+                          IndexingOptions IndexOpts,
+                          RecordingOptions RecordOpts, StringRef OutputFile,
+                          const FileEntry *RootFile, Module *UnitModule,
+                          StringRef SysrootPath) {
+
+  // TODO persist collected index data
+}
+
+static std::unique_ptr<FrontendAction>
+createIndexDataRecordingAction(IndexingOptions IndexOpts,
+                               RecordingOptions RecordOpts,
+                               std::unique_ptr<FrontendAction> WrappedAction) {
+  if (WrappedAction)
+    return llvm::make_unique<WrappingIndexRecordAction>(
+        std::move(WrappedAction), std::move(IndexOpts), std::move(RecordOpts));
+  return llvm::make_unique<IndexRecordAction>(std::move(IndexOpts),
+                                              std::move(RecordOpts));
+}
+
+static std::pair<IndexingOptions, RecordingOptions>
+getIndexOptionsFromFrontendOptions(const FrontendOptions &FEOpts) {
+  index::IndexingOptions IndexOpts;
+  index::RecordingOptions RecordOpts;
+  RecordOpts.DataDirPath = FEOpts.IndexStorePath;
+  if (FEOpts.IndexIgnoreSystemSymbols) {
+    IndexOpts.SystemSymbolFilter =
+        index::IndexingOptions::SystemSymbolFilterKind::None;
+  }
+  RecordOpts.RecordSymbolCodeGenName = FEOpts.IndexRecordCodegenName;
+  return {IndexOpts, RecordOpts};
+}
+
+std::unique_ptr<FrontendAction> index::createIndexDataRecordingAction(
+    const FrontendOptions &FEOpts,
+    std::unique_ptr<FrontendAction> WrappedAction) {
+  auto IndexAndRecordOpts = getIndexOptionsFromFrontendOptions(FEOpts);
+  return ::createIndexDataRecordingAction(IndexAndRecordOpts.first,
+                                          IndexAndRecordOpts.second,
+                                          std::move(WrappedAction));
+}

lib/Index/IndexingContext.h

 //===- IndexingContext.h - Indexing context data ----------------*- 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_LIB_INDEX_INDEXINGCONTEXT_H
 #define LLVM_CLANG_LIB_INDEX_INDEXINGCONTEXT_H
 
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Index/IndexingAction.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 
 namespace clang {
   class ASTContext;
   class Decl;
   class DeclGroupRef;
   class ImportDecl;
   class TagDecl;
   class TypeSourceInfo;
   class NamedDecl;
   class ObjCMethodDecl;
   class DeclContext;
   class NestedNameSpecifierLoc;
   class Stmt;
   class Expr;
   class TypeLoc;
-  class SourceLocation;
+  class DirectoryEntry;
 
 namespace index {
   class IndexDataConsumer;
 
+/// Tracks the current system root path and computes and caches whether a
+/// file is considered a system file or not
+class IsSystemFileCache {
+  std::string SysrootPath;
+  // Records whether a directory entry is system or not.
+  llvm::DenseMap<const DirectoryEntry *, bool> DirEntries;
+  // Keeps track of the last check for whether a FileID is system or
+  // not. This is used to speed up isSystemFile() call.
+  std::pair<FileID, bool> LastFileCheck;
+
+public:
+  IsSystemFileCache() = default;
+  IsSystemFileCache(std::string SysrootPath);
+
+  void setSysrootPath(StringRef path);
+  StringRef getSysrootPath() const { return SysrootPath; }
+  bool isSystem(FileID FID, SourceManager &SM);
+};
+
+/// Generates and reports indexing data to the provided \c IndexDataConsumer
+/// for any AST nodes passed to its various \c index* methods.
 class IndexingContext {
   IndexingOptions IndexOpts;
   IndexDataConsumer &DataConsumer;
+  IsSystemFileCache &IsSystemCache;
   ASTContext *Ctx = nullptr;
 
 public:
-  IndexingContext(IndexingOptions IndexOpts, IndexDataConsumer &DataConsumer)
-    : IndexOpts(IndexOpts), DataConsumer(DataConsumer) {}
+  IndexingContext(IndexingOptions IndexOpts, IndexDataConsumer &DataConsumer,
+                  IsSystemFileCache &IsSystemCache)
+      : IndexOpts(IndexOpts), DataConsumer(DataConsumer),
+        IsSystemCache(IsSystemCache) {}
 
   const IndexingOptions &getIndexOpts() const { return IndexOpts; }
   IndexDataConsumer &getDataConsumer() { return DataConsumer; }
 
   void setASTContext(ASTContext &ctx) { Ctx = &ctx; }
 
   bool shouldIndex(const Decl *D);
 

lib/Index/IndexingContext.cpp

 //===- IndexingContext.cpp - Indexing context data ------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "IndexingContext.h"
-#include "clang/Index/IndexDataConsumer.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclObjC.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Index/IndexDataConsumer.h"
+#include "llvm/Support/Path.h"
 
 using namespace clang;
 using namespace index;
+using namespace llvm;
 
 static bool isGeneratedDecl(const Decl *D) {
   if (auto *attr = D->getAttr<ExternalSourceSymbolAttr>()) {
     return attr->getGeneratedDeclaration();
   }
   return false;
 }
 
+void IsSystemFileCache::setSysrootPath(llvm::StringRef path) {
+  // Ignore sysroot path if it points to root, otherwise every header will be
+  // treated as system one.
+  if (sys::path::root_path(path) == path)
+    path = StringRef();
+  SysrootPath = path;
+}
+
+IsSystemFileCache::IsSystemFileCache(std::string path) { setSysrootPath(path); }
+
+bool IsSystemFileCache::isSystem(clang::FileID FID, clang::SourceManager &SM) {
+  if (LastFileCheck.first == FID)
+    return LastFileCheck.second;
+
+  auto result = [&](bool res) -> bool {
+    LastFileCheck = {FID, res};
+    return res;
+  };
+
+  bool Invalid = false;
+  const SrcMgr::SLocEntry &SEntry = SM.getSLocEntry(FID, &Invalid);
+  if (Invalid || !SEntry.isFile())
+    return result(false);
+
+  const SrcMgr::FileInfo &FI = SEntry.getFile();
+  if (FI.getFileCharacteristic() != SrcMgr::C_User)
+    return result(true);
+
+  auto *CC = FI.getContentCache();
+  if (!CC)
+    return result(false);
+  auto *FE = CC->OrigEntry;
+  if (!FE)
+    return result(false);
+
+  if (SysrootPath.empty())
+    return result(false);
+
+  // Check if directory is in sysroot so that we can consider system headers
+  // even the headers found via a user framework search path, pointing inside
+  // sysroot.
+  auto dirEntry = FE->getDir();
+  auto pair = DirEntries.insert(std::make_pair(dirEntry, false));
+  bool &isSystemDir = pair.first->second;
+  bool wasInserted = pair.second;
+  if (wasInserted) {
+    isSystemDir = StringRef(dirEntry->getName()).startswith(SysrootPath);
+  }
+  return result(isSystemDir);
+}
+
 bool IndexingContext::shouldIndex(const Decl *D) {
   return !isGeneratedDecl(D);
 }
 
 const LangOptions &IndexingContext::getLangOpts() const {
   return Ctx->getLangOpts();
 }
 
if (Loc.isInvalid())
     return true;
 
   FileID FID;
   unsigned Offset;
   std::tie(FID, Offset) = SM.getDecomposedLoc(Loc);
   if (FID.isInvalid())
     return true;
 
-  bool Invalid = false;
-  const SrcMgr::SLocEntry &SEntry = SM.getSLocEntry(FID, &Invalid);
-  if (Invalid || !SEntry.isFile())
-    return true;
-
-  if (SEntry.getFile().getFileCharacteristic() != SrcMgr::C_User) {
+  bool IsInSystemFile = IsSystemCache.isSystem(FID, SM);
+  if (IsInSystemFile) {
     switch (IndexOpts.SystemSymbolFilter) {
     case IndexingOptions::SystemSymbolFilterKind::None:
       return true;
     case IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly:
     case IndexingOptions::SystemSymbolFilterKind::All:
       break;
     }
   }
 
   SymbolRoleSet Roles = (unsigned)SymbolRole::Declaration;
   if (ImportD->isImplicit())
     Roles |= (unsigned)SymbolRole::Implicit;
 
-  return DataConsumer.handleModuleOccurence(ImportD, Roles, FID, Offset);
+  return DataConsumer.handleModuleOccurence(ImportD, Roles, FID, Offset,
+                                            IsInSystemFile);
 }
 
 bool IndexingContext::isTemplateImplicitInstantiation(const Decl *D) {
   TemplateSpecializationKind TKind = TSK_Undeclared;
   if (const ClassTemplateSpecializationDecl *
       SD = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
     TKind = SD->getSpecializationKind();
   } else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
bool IndexingContext::shouldIgnoreIfImplicit(const Decl *D) {
   if (isa<ImportDecl>(D))
     return false;
   return true;
 }
 
 static const CXXRecordDecl *
 getDeclContextForTemplateInstationPattern(const Decl *D) {
   if (const auto *CTSD =
           dyn_cast<ClassTemplateSpecializationDecl>(D->getDeclContext()))
     return CTSD->getTemplateInstantiationPattern();
   else if (const auto *RD = dyn_cast<CXXRecordDecl>(D->getDeclContext()))
     return RD->getInstantiatedFromMemberClass();
   return nullptr;
 }
 
 static const Decl *adjustTemplateImplicitInstantiation(const Decl *D) {
   if (const ClassTemplateSpecializationDecl *
if (const auto *ED = dyn_cast<EnumDecl>(ECD->getDeclContext())) {
         for (const NamedDecl *BaseECD : Pattern->lookup(ECD->getDeclName()))
           return BaseECD;
       }
     }
   }
   return nullptr;
 }
 
-static bool isDeclADefinition(const Decl *D, const DeclContext *ContainerDC, ASTContext &Ctx) {
+static bool isDeclADefinition(const Decl *D, const DeclContext *ContainerDC,
+                              ASTContext &Ctx) {
   if (auto VD = dyn_cast<VarDecl>(D))
     return VD->isThisDeclarationADefinition(Ctx);
 
   if (auto FD = dyn_cast<FunctionDecl>(D))
     return FD->isThisDeclarationADefinition();
 
   if (auto TD = dyn_cast<TagDecl>(D))
     return TD->isThisDeclarationADefinition();

 
 bool IndexingContext::handleDeclOccurrence(const Decl *D, SourceLocation Loc,
                                            bool IsRef, const Decl *Parent,
                                            SymbolRoleSet Roles,
                                            ArrayRef<SymbolRelation> Relations,
                                            const Expr *OrigE,
                                            const Decl *OrigD,
                                            const DeclContext *ContainerDC) {
-  if (D->isImplicit() && !isa<ObjCMethodDecl>(D))
+  if (D->isImplicit() && !(isa<ObjCMethodDecl>(D) || isa<ObjCIvarDecl>(D)))
     return true;
   if (!isa<NamedDecl>(D) || shouldSkipNamelessDecl(cast<NamedDecl>(D)))
     return true;
 
   SourceManager &SM = Ctx->getSourceManager();
   Loc = SM.getFileLoc(Loc);
   if (Loc.isInvalid())
     return true;
 
   FileID FID;
   unsigned Offset;
   std::tie(FID, Offset) = SM.getDecomposedLoc(Loc);
   if (FID.isInvalid())
     return true;
 
-  bool Invalid = false;
-  const SrcMgr::SLocEntry &SEntry = SM.getSLocEntry(FID, &Invalid);
-  if (Invalid || !SEntry.isFile())
-    return true;
-
-  if (SEntry.getFile().getFileCharacteristic() != SrcMgr::C_User) {
+  bool IsInSystemFile = IsSystemCache.isSystem(FID, SM);
+  if (IsInSystemFile) {
     switch (IndexOpts.SystemSymbolFilter) {
     case IndexingOptions::SystemSymbolFilterKind::None:
       return true;
     case IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly:
       if (!shouldReportOccurrenceForSystemDeclOnlyMode(IsRef, Roles, Relations))
         return true;
       break;
     case IndexingOptions::SystemSymbolFilterKind::All:
bool IndexingContext::handleDeclOccurrence(const Decl *D, SourceLocation Loc,
 
   for (auto &Rel : Relations) {
     addRelation(SymbolRelation(Rel.Roles,
                                Rel.RelatedSymbol->getCanonicalDecl()));
   }
 
   IndexDataConsumer::ASTNodeInfo Node{ OrigE, OrigD, Parent, ContainerDC };
   return DataConsumer.handleDeclOccurence(D, Roles, FinalRelations, FID, Offset,
-                                          Node);
+                                          IsInSystemFile, Node);
 }

test/Index/Store/assembly-invocation.c

+// Make sure it doesn't crash.
+// RUN: %clang -target x86_64-apple-macosx10.7 -S %s -o %t.s
+// RUN: %clang -target x86_64-apple-macosx10.7 -c %t.s -o %t.o -index-store-path %t.idx

tools/c-index-test/core_main.cpp

public:
   PrintIndexDataConsumer(raw_ostream &OS) : OS(OS) {
   }
 
   void initialize(ASTContext &Ctx) override {
     CGNameGen.reset(new CodegenNameGenerator(Ctx));
   }
 
   bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
-                           ArrayRef<SymbolRelation> Relations,
-                           FileID FID, unsigned Offset,
+                           ArrayRef<SymbolRelation> Relations, FileID FID,
+                           unsigned Offset, bool IsInSystemFile,
                            ASTNodeInfo ASTNode) override {
     ASTContext &Ctx = D->getASTContext();
     SourceManager &SM = Ctx.getSourceManager();
 
     unsigned Line = SM.getLineNumber(FID, Offset);
     unsigned Col = SM.getColumnNumber(FID, Offset);
     OS << Line << ':' << Col << " | ";
 
for (auto &SymRel : Relations) {
       printSymbolNameAndUSR(SymRel.RelatedSymbol, Ctx, OS);
       OS << '\n';
     }
 
     return true;
   }
 
   bool handleModuleOccurence(const ImportDecl *ImportD, SymbolRoleSet Roles,
-                             FileID FID, unsigned Offset) override {
+                             FileID FID, unsigned Offset,
+                             bool IsInSystemFile) override {
     ASTContext &Ctx = ImportD->getASTContext();
     SourceManager &SM = Ctx.getSourceManager();
 
     unsigned Line = SM.getLineNumber(FID, Offset);
     unsigned Col = SM.getColumnNumber(FID, Offset);
     OS << Line << ':' << Col << " | ";
 
     printSymbolInfo(getSymbolInfo(ImportD), OS);

tools/diagtool/DiagnosticNames.cpp

 #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"
 #include "clang/Basic/DiagnosticAnalysisKinds.inc"
 #include "clang/Basic/DiagnosticRefactoringKinds.inc"
+#include "clang/Basic/DiagnosticIndexKinds.inc"
 #undef DIAG
 };
 
 static bool orderByID(const DiagnosticRecord &Left,
                       const DiagnosticRecord &Right) {
   return Left.DiagID < Right.DiagID;
 }
 

tools/libclang/CXIndexDataConsumer.h

public:
   CXIdxClientEntity getClientEntity(const Decl *D) const;
   void setClientEntity(const Decl *D, CXIdxClientEntity client);
 
   static bool isTemplateImplicitInstantiation(const Decl *D);
 
 private:
   bool handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
                            ArrayRef<index::SymbolRelation> Relations,
-                           FileID FID, unsigned Offset,
+                           FileID FID, unsigned Offset, bool IsInSystemFile,
                            ASTNodeInfo ASTNode) override;
 
   bool handleModuleOccurence(const ImportDecl *ImportD,
-                             index::SymbolRoleSet Roles,
-                             FileID FID, unsigned Offset) override;
+                             index::SymbolRoleSet Roles, FileID FID,
+                             unsigned Offset, bool IsInSystemFile) override;
 
   void finish() override;
 
   bool handleDecl(const NamedDecl *D,
                   SourceLocation Loc, CXCursor Cursor,
                   DeclInfo &DInfo,
                   const DeclContext *LexicalDC = nullptr,
                   const DeclContext *SemaDC = nullptr);

tools/libclang/CXIndexDataConsumer.cpp

public:
 
   bool VisitImportDecl(const ImportDecl *D) {
     DataConsumer.importedModule(D);
     return true;
   }
 };
 }
 
-bool CXIndexDataConsumer::handleDeclOccurence(const Decl *D,
-                                              SymbolRoleSet Roles,
-                                             ArrayRef<SymbolRelation> Relations,
-                                              FileID FID, unsigned Offset,
-                                              ASTNodeInfo ASTNode) {
+bool CXIndexDataConsumer::handleDeclOccurence(
+    const Decl *D, SymbolRoleSet Roles, ArrayRef<SymbolRelation> Relations,
+    FileID FID, unsigned Offset, bool IsInSystemFile, ASTNodeInfo ASTNode) {
   SourceLocation Loc = getASTContext().getSourceManager()
       .getLocForStartOfFile(FID).getLocWithOffset(Offset);
 
   if (Roles & (unsigned)SymbolRole::Reference) {
     const NamedDecl *ND = dyn_cast<NamedDecl>(D);
     if (!ND)
       return true;
 
if (Roles & (unsigned)SymbolRole::Reference) {
     }
     IndexingDeclVisitor(*this, Loc, LexicalDC).Visit(ASTNode.OrigD);
   }
 
   return !shouldAbort();
 }
 
 bool CXIndexDataConsumer::handleModuleOccurence(const ImportDecl *ImportD,
-                                                SymbolRoleSet Roles,
-                                                FileID FID,
-                                                unsigned Offset) {
+                                                SymbolRoleSet Roles, FileID FID,
+                                                unsigned Offset,
+                                                bool IsInSystemFile) {
   IndexingDeclVisitor(*this, SourceLocation(), nullptr).Visit(ImportD);
   return !shouldAbort();
 }
 
 void CXIndexDataConsumer::finish() {
   indexDiagnostics();
 }