[clangd] #include statements support for Open definition

[Nebiroth]

Summary

ctrl-clicking on #include statements now opens the file being pointed by that statement.

[ilya-biryukov]

clangd/ClangdUnit.cpp

162

Please use our descriptive Path typedef.

unittests/clangd/ClangdTests.cpp

991 ↗

(On Diff #118046)

Some includes may come outside of preamble, I would expect current implementation to fail for this case:

#include <vector> // <-- works here 
// preamble ends here
int main() {

  #include "my_include.h" // <-- does not work here
}

[Nebiroth]

clangd/ClangdUnit.cpp

149

If I were to use InclusionDirective , how would that callback be called automatically? As far as I know, it wouldn't be called automatically for every file that gets indexed the same way AfterExecute would be.

[ilya-biryukov]

clangd/ClangdUnit.cpp

149

It will be called for each #include directive that Preprocessor encountered while building the preamble.
A separate instance of CppFilePreambleCallbacks is created every time we build a preamble for file.

Does that answer your question?

[Nebiroth]

• Now overriding InclusionDirective as a callback to get proper includes information.
• Fixed tests.

[Nebiroth]

• Fixed adding incorrect test file.

This revision requires modifications to clang which can be reviewed on revision D39375.

[malaperle]

I'm not sure it's better to use the InclusionDirective callback for this. We need to get the includes in two places: in the preamble and non-preamble. In the preamble we use the callback, have to store some temporary stuff because we don't have a SourceManager in InclusionDirective, then in finish we use the SourceManager to convert everything. In the non-preamble, we cannot use the callback so we use the SourceManager to go through the includes. Therefore, we have to maintain two different ways of getting the inclusion map. Without using the InclusionDirective callback, we use the SourceManager in both cases and can use the same code to iterate through inclusions, just on two different SourceManagers at different moments. We also don't need to make another patch to modify the PreambleCallbacks. I also double this is much slower but I think this simplifies the code nicely.
What do you think?

[ilya-biryukov]

In D38639#920427, @malaperle wrote:

I'm not sure it's better to use the InclusionDirective callback for this. We need to get the includes in two places: in the preamble and non-preamble. In the preamble we use the callback, have to store some temporary stuff because we don't have a SourceManager in InclusionDirective, then in finish we use the SourceManager to convert everything. In the non-preamble, we cannot use the callback so we use the SourceManager to go through the includes. Therefore, we have to maintain two different ways of getting the inclusion map. Without using the InclusionDirective callback, we use the SourceManager in both cases and can use the same code to iterate through inclusions, just on two different SourceManagers at different moments. We also don't need to make another patch to modify the PreambleCallbacks. I also double this is much slower but I think this simplifies the code nicely.
What do you think?

I think we should never iterate through SourceManager, as it's much easier to get wrong than using the callbacks. I agree that all that fiddling with callbacks is unfortunate, but it's well worth the fact that it'd be much easier to tell that the implementation is correct by simply looking at the implementation and not knowing how SourceManager works. PPCallbacks is a much more direct API that was designed to handle our purposes.

Note that we don't need to use SourceManager to find non-preamble includes, we should implement proper PPCallbacks and use them when building the AST.

[ilya-biryukov]

clangd/ClangdServer.cpp

368 ↗

(On Diff #120485)

This map and two vectors come up quite often in your commit.
Is this a storage for saved includes? Could we define a class that stores this data?

Something like

class IncludeReferenceMap {
  llvm::Optional<PathRef> findIncludeTargetAtLoc(Location Loc);

private:
// maps, vectors, whatever we need go here ....
};

We'd build it using the callbacks, then store it in PreambleData (for PCH include) and ParsedAST (for non-PCH includes). And later query both maps to get the result.

clangd/ClangdUnit.cpp

31

We probably don't need it this include

35

What's the purpose of this class?

157

Please also use PPCallbacks interface instead of the SourceManager (should be possible to hook it up somewhere in ParsedAST::Build).

[malaperle]

In D38639#920487, @ilya-biryukov wrote:

I think we should never iterate through SourceManager, as it's much easier to get wrong than using the callbacks. I agree that all that fiddling with callbacks is unfortunate, but it's well worth the fact that it'd be much easier to tell that the implementation is correct by simply looking at the implementation and not knowing how SourceManager works. PPCallbacks is a much more direct API that was designed to handle our purposes.

Note that we don't need to use SourceManager to find non-preamble includes, we should implement proper PPCallbacks and use them when building the AST.

Sounds good!

[Nebiroth]

clangd/ClangdUnit.cpp

35

We need to be able to use a wrapper class to be able to make a unique_ptr to be sent to PrecompiledPreamble::Build in order to add the list of preprocessor Callbacks.

[Nebiroth]

Using PPCallbacks interface to find non-preamble includes
Created inner class to store vectors in to find locations
Refactored methods to remove some unnecessary parameters
Refactored Unit tests
Merge with most recent master branch + clang

[Nebiroth]

Fixed re-added include

[ilya-biryukov]

clangd/ClangdUnit.cpp

35

Could we implement an instance of PPCallbacks that contains CppFilePreambleCallbacks and forwards to that specific method instead?

The reason is that we're not really delegating other methods in this calls(nor should we, the implementation would be too compilcated).
Having a class that contains CppFilePreambleCallbacks &Collector and calling Collector.InclusionDirective seems perfectly fine, though: its purpose is clear and the implementation is easy.

[Nebiroth]

Creating unique_ptr for wrapper class now uses overriden method createPPCallbacks() from PrecompiledPreamble class
Moved wrapper class to inline inside createPPCallbacks()
Wrapper class now uses CppFilePreambleCallbacks as field

[malaperle]

clangd/ClangdServer.cpp

454 ↗

(On Diff #126429)

IncludeReferenceMap & here? See other comment in takeIRM

clangd/ClangdUnit.cpp

31

remove

121

remove

128

These need to be swapped to be in the same order that they are declared below.

130

I don't think we need this if we pass the map by reference (and store it as a reference, see other comment)

137

Remove (see previous comment)

140

remove

157

I tried to simply the methods introduced here:

void AfterExecute(CompilerInstance &CI) override {
  SourceMgr = &CI.getSourceManager();
  for (auto InclusionLoc : TempPreambleIncludeLocations)
    addIncludeLocation(InclusionLoc);
}

void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                        StringRef FileName, bool IsAngled,
                        CharSourceRange FilenameRange, const FileEntry *File,
                        StringRef SearchPath, StringRef RelativePath,
                        const Module *Imported) override {
  auto SR = FilenameRange.getAsRange();
  if (SR.isInvalid() || !File || File->tryGetRealPathName().empty())
    return;

  if (SourceMgr) {
    addIncludeLocation({SR, File->tryGetRealPathName()});
  } else {
    // When we are processing the inclusion directives inside the preamble,
    // we don't have access to a SourceManager, so we cannot convert
    // SourceRange to Range. This is done instead in AfterExecute when a
    // SourceManager becomes available.
    TempPreambleIncludeLocations.push_back({SR, File->tryGetRealPathName()});
  }
}

void addIncludeLocation(std::pair<SourceRange, std::string> InclusionLoc) {
  // Only inclusion directives in the main file make sense. The user cannot
  // select directives not in the main file.
  if (SourceMgr->getMainFileID() == SourceMgr->getFileID(InclusionLoc.first.getBegin()))
    IRM.insert({getRange(InclusionLoc.first), InclusionLoc.second});
}

Range getRange(SourceRange SR) {
  Position Begin;
  Begin.line = SourceMgr->getSpellingLineNumber(SR.getBegin());
  Begin.character = SourceMgr->getSpellingColumnNumber(SR.getBegin());
  Position End;
  End.line = SourceMgr->getSpellingLineNumber(SR.getEnd());
  End.character = SourceMgr->getSpellingColumnNumber(SR.getEnd());
  return {Begin, End};
}

162

IncludeReferenceMap &IRM;

That way, we can use the same map and pass it multiple times to different "collectors".

179

I think we need a "is main file" check here. In case this is used on a AST with no preamble.

273

I don't think this change was brought back intentionally?

283–284

IncludeReferenceMap &IRM

311

unnecessary std::move

328

std::move(IRM)

339–340

I think after rebase, all this code in DeclarationLocationsFinder will need to be moved around, probably inside clangd::findDefinitions?

341

for (auto &IncludeLoc : IncludeLocationMap) {
  Range R = IncludeLoc.first;
  if (isSameLine(R.start.line))
    addLocation(URI::fromFile(IncludeLoc.second), R);
}

348

Probably if an include location was found, we don't want to continue and it should early return.

474

I think you can put the construction of SerializedDeclsCollector back to where it was.

479

You can pass just IRM, so there is no need for the getIRM. We'll have eventually to transfer ownership to ParsedAST so maybe std::move(IRM) here?

clangd/ClangdUnit.h

63

With the map being the only field left (see other comments), there is no need for a class I think. Maybe make a handy type alias for this?

using IncludeReferenceMap = std::unordered_map<Range, Path, RangeHash>;

64

findIncludeTargetAtLoc is never used, remove

68

No need to keep this.

69

No need to keep this.

101

We need to keep this copy of IRM alive for subsequent calls to "open definition" for the same ParsedAST. So it is correct to not do a std::move here. However, I think it should be changed to:

IncludeReferenceMap &getIRM()

and also the caller should assign to a reference.

131

this wrongly came back?

unittests/clangd/ClangdTests.cpp

622 ↗

(On Diff #126429)

why is this check removed?

751 ↗

(On Diff #126429)

Need to test includes outside preamble.

778 ↗

(On Diff #126429)

I get an "unchecked assertion" here before the Expected is not checked. I was able to replace it with:

auto ExpectedLocations = Server.findDefinitions(FooCpp, P);
ASSERT_TRUE(!!ExpectedLocations);
std::vector<Location> Locations = ExpectedLocations->Value;

784 ↗

(On Diff #126429)

I get a test failure here.

../tools/clang/tools/extra/unittests/clangd/ClangdTests.cpp(785): Error:       Expected: check
      Which is: "clangd-test/foo."
To be equal to: FooH
      Which is: { '/' (47, 0x2F), 'c' (99, 0x63), 'l' (108, 0x6C), 'a' (97, 0x61), 'n' (110, 0x6E), 'g' (103, 0x67), 'd' (100, 0x64), '-' (45, 0x2D), 't' (116, 0x74), 'e' (101, 0x65), 's' (115, 0x73), 't' (116, 0x74), '/' (47, 0x2F), 'f' (102, 0x66), 'o' (111, 0x6F), 'o' (111, 0x6F), '.' (46, 0x2E), 'h' (104, 0x68) }

Not sure what's wrong. There is an additional front slash and missing ".h" at the end?

793 ↗

(On Diff #126429)

Similar unchecked problem here, this works (although a bit yucky):

ExpectedLocations = Server.findDefinitions(FooCpp, P3);
ASSERT_TRUE(!!ExpectedLocations);
Locations = ExpectedLocations->Value;
EXPECT_TRUE(!Locations.empty());

// Test invalid include
Position P2 = Position{2, 11};

ExpectedLocations = Server.findDefinitions(FooCpp, P2);
ASSERT_TRUE(!!ExpectedLocations);
Locations = ExpectedLocations->Value;
EXPECT_TRUE(Locations.empty());

[Nebiroth]

inner class IncludeReferenceMap replaced by one map
fillRangeVector() and findPreambleIncludes() code moved into wrapper class
Open definition now returns an empty Range struct (line, character = 0)
Fixed unit tests
Minor code improvements

[ilya-biryukov]

clangd/ClangdServer.cpp

446 ↗

(On Diff #127161)

We don't need this Preamble variable, right?

clangd/ClangdUnit.cpp

121

We don't need to derive this class from PPCallbacks, DelegatingPPCallbacks can call any method on it anyway as it knows the exact type. Please remove this inheritance.

127

Instead of passing SourceManager in constructor, maybe add a BeforeExecute(CompilerInstance &CI) method to PreambleCallbacks and set SourceManager when its called.

164

Adding BeforeExecute would allow to get rid of this vector.

214

NIT: replace with return llvm::make_unique<DelegatingPPCallbacks>(*this); and remove the local variable.

283–284

Let's store IncludeReferenceMap in ParsedAST instead of having it as out parameter.
It allows to properly manage the lifetime of IncludeReferenceMap (logically, it is exactly tied to the AST).

308

Why do we use CppFilePreambleCallbacks here? Factor the code that handles the references into a separate class and use it here:

class IncludeRefsCollector : public PPCallbacks {
public:
  IncludeRefsCollector(IncludeReferenceMap &Refs) : Refs(Refs) {}

   // implementation of PPCallbacks ...

private;
  IncludeReferenceMap &Refs;
};

/// .....
class CppFilePreambleCaallbacks {
public:
// ....

  std::unique_ptr<PPCallbacks> createPPCallbacks() { return llvm::make_unique<IncludeRefsCollector>(this->IRM); }
};

// ....
void ParsedAST::Build() {
  // ....
  // No need to create CppFilePreambleCallbacks here.
  Clang->getPreprocessor().addPPCallbacks(llvm::make_unique<IncludeRefsCollector>(IRM));
  //....
}

340

Use IncludeReferenceMap typedef here.

340

Do we change semantics of getting the file paths here? Maybe leave the code as is?

340

NIT: empty linke at the start of the function

342

• use a typedef for the map here
• accept the map by const reference to avoid copies

343

Let's check the the point is in range, not that it's not the same line. So that we don't trigger goto in strange places, like comments:

#include <vector> // this include is very important.  <--- we should not skip to include when editor caret is over a comment

343

If we store IncludeLocationMap in ParsedAST we don't need to change interface of this function.

345

This method does not seem particularly useful. One can construct location like this: Location{Uri, R}, the whole method can be written as DeclarationLocations.push_back(Location{Uri, R}).

clangd/ClangdUnit.h

63

We store a map, but only iterate through it in order. Let's use vector<pair<Range, Path>> instead.

[Nebiroth]

CppFilePreambleCallbacks no longer extends PPCallbacks
CppFilePreambleCallbacks no longer needs SourceManager parameter
Removed temporary vector TempPreambleIncludeLocations
IncludeReferenceMap in ParsedAST no longer passed by reference
Code handling includes outside of preambles is now separated in IncludeRefsCollector class
Removed addLocation
Changed isSameLine function name and now checks if searched location is in range instead of verifying the line
Removed changes to findDefinitions interface

[ilya-biryukov]

Thanks for addressing the comments quickly.
I took another look and added a few more comments.
This moves in the right direction, though, this is really close to landing.

clangd/ClangdServer.cpp

25 ↗

(On Diff #127386)

Is this include redundant now? Can we remove it?

430 ↗

(On Diff #127386)

Capturing this seems redundant. Remove it?

clangd/ClangdUnit.cpp

90

clang-format please

90

Do we need both checks? Doesn't SourceMgr.isInMainFile handles all the cases?

127

Let's create a new empty map inside this class and have a takeIncludeReferences method (similar to TopLevelDeclIDs and takeTopLevelDeclIDs)

164

We should have SourceMgr at all the proper places now, let's store IncludeReferenceMap directly

285

What reference does this IncludeReferenceMap contain? Includes from the preamble?

339–340

This class handles processing AST and preprocessor, it does not need to get IncludeLocationMap in constructor or store it at all.
Remove IncludeLocationMap from this class and handle getting references from IncludeLocationMap outside this class instead.

clangd/ClangdUnit.h

101

Make it all const?

const IncludeReferenceMap &getIRM() const { return IRM; }

185

This function moved without need and lost a comment.

[Nebiroth]

clangd/ClangdUnit.cpp

90

This check was here to prevent an issue that appeared previously. I don't think this check is needed anymore.

164

The idea was to pass a single map around to fill with every reference instead of having separate maps being merged together.

285

This should contain every reference available for one file.

[Nebiroth]

Removed some useless inclusions
Removed superfluous check when inserting data in map
Moved addition to DeclarationLocations in finish() outside of DeclMacrosFinder
Merged with revision 321087 (moved findDefinitions and findDocumentHighlights

[ilya-biryukov]

Another round of review

clangd/ClangdUnit.cpp

127

This comment is not addressed yet, but marked as done.

154

Maybe add assert(SourceMgr && "SourceMgr must be set at this point") here?

164

You copy the map for preamble and then append to it in CppFilePreambleCallbacks? That also LG, we should not have many references there anyway.

285

Thanks for clarifying, LG.

289

Don't we already store the map we need in PreambleData? Why do we need an extra IRM parameter?

clangd/ClangdUnit.h

185

Now we have duplicated definitions here. Bad merge?

clangd/CodeComplete.cpp

328 ↗

(On Diff #127617)

Accidental change?

807 ↗

(On Diff #127617)

Accidental change?

clangd/XRefs.cpp

58

We don't need locations anymore. Remove them.

175

Probably a good place for a comment. Also, maybe rename local var to something easier to understand like IncludeDefinitions

/// Process targets for paths inside #include directive.
std::vector<Location> IncludeTargets;

176

No need to special case empty maps, remove the if altogether.

181

Replace with DeclMacrosFinder->getSearchedLocation() SourceLocationBeg, it makes the code easier to read.

183

why do we need to convert to unsigned? To slience compiler warnings?

186

this should be R.start.charater <= CharNumber && CharNumber <= R.end.character

[ilya-biryukov]

clangd/XRefs.cpp

181

Sorry, the comment got messed up. Here's the correct version:

Replace DeclMacrosFinder->getSearchedLocation() with SourceLocationBeg, it makes the code easier to read.

[Nebiroth]

Minor code cleanup

[malaperle]

@ilya-biryukov Hi! I'll be updating William's patches that were in progress. I just have a few comments/question before I send a new update. (I also don't know if I can update this diff or I have to create a new diff on Phabricator... I guess we'll see!!).

clangd/ClangdUnit.cpp

127

As mentioned below, the idea was to have a single map being appended to, without having to merge two separate maps. However, I can change the code so that two separate maps are built and merged if you prefer.

But I'm not so clear if that's what you'd prefer:

You copy the map for preamble and then append to it in CppFilePreambleCallbacks? That also LG, we should not have many references there anyway.

It's not meant to have any copy. The idea was to create a single IncludeReferenceMap in CppFile::deferRebuild, populate it with both preamble and non-preamble include references and std::move it around for later use (stored in ParsedAST).

289

Since the map will be now stored in ParsedAST and the instance doesn't exist yet, we need to keep the IRM parameter. I noticed that it wasn't being std::move'd though.

clangd/XRefs.cpp

183

Yes, "line" from the protocol is signed, whereas getSpellingColumn/lineNumber returns unsigned. I'll extract another var for the line number and cast both to int instead to have less casts and make the condition smaller.

[ilya-biryukov]

@malaperle, hi! Both new diff and updating this works, so feel free the one that suits you best. I tend to look over the whole change on each new round of reviews anyway.

clangd/ClangdUnit.cpp

127

We can't have a single map because AST is rebuilt more often than the Preamble, so we have two options:

• Store a map for the preamble separately, copy it when we need to rebuild the AST and append references from the AST to the new instance of the map.
• Store two maps: one contains references only from the Preamble, the other one from the AST.

I think both are fine, since the copy of the map will be cheap anyway, as we only store a list of includes inside the main file.

289

That looks fine. Also see the other comment on why we need to copy the map from the Preamble, and not std::move it.

clangd/XRefs.cpp

183

Can we maybe convert to clangd::Position using the helper methods first and do the comparison of two clangd::Positions?
Comparing between clangd::Position and clang's line/column numbers is a common source of off-by-one errors in clangd.

[malaperle]

clangd/ClangdUnit.cpp

127

We can't have a single map because AST is rebuilt more often than the Preamble, so we have two options:

Doh! Indeed.

OK so I added the map in PreambleData, this one will say every reparse unless the preamble changes. When the AST is built/rebuilt, the map is copied (or empty) and includes from the AST are appended to it then stored in ParsedAST (option #1?).

clangd/XRefs.cpp

183

offsetToPosition (if that's what you mean) uses a StringRef of the code, which is not handy in this case. I'll add another one "sourceLocToPosition" to convert a SourceLocation to a Position. WDYT? It can be used in a few other places too.

[malaperle]

Store map in PremableData and copy it on reparse.
Convert SourceLoc to Position when comparing with include Positions.

[malaperle]

Revert an unintentional white space change.

[ilya-biryukov]

clangd/ClangdUnit.h

48

We use unordered_map as a vector<pair<>> here. (i.e. we never look up values by key, because we don't only the range, merely a point in the range)
Replace map with vector<pair<>> and remove RangeHash that we don't need anymore.

clangd/XRefs.cpp

183

Looks good, thanks!

unittests/clangd/ClangdTests.cpp

781 ↗

(On Diff #129293)

We moved findDefinition tests to XRefsTests.cpp and added a useful Annotation helper to make writing these tests simpler.
Maybe we could use it for this test as well?

[malaperle]

Move tests to XRefsTests, change IncludeReferenceMap to a vector and rename it.

[malaperle]

clangd/ClangdUnit.h

48

Done. I also renamed, IncludeReferenceMap to InclusionLocations. I thought I was more suitable.

unittests/clangd/ClangdTests.cpp

781 ↗

(On Diff #129293)

Done. (I don't know why I can't use the "done" checkbox, maybe because I wasn't the original author?)

[ilya-biryukov]

clangd/ClangdUnit.cpp

102

NIT: replace FilenameRange.getAsRange() with SR

131

NIT: remove ctor, use initializer on the member instead:

private:
  SourceManager *SourceMgr = nullptr;

163

Maybe swap IncLocations and SourceMgr? Grouping TopLevelDecls, TopLevelDeclIDs and IncLocations together seems reasonable, as all of them are essentially output parameters.

287

NIT: move IncLocations closer to their first use, i.e. create the variable right before addPPCallbacks()

clangd/ClangdUnit.h

48

LG

105

NIT: move to .cpp file to be consisten with other decls in this file.

clangd/XRefs.cpp

179

NIT: SourceMgr does not depend on the loop variable, declare it out of the loop.

182

NIT: define operator <= for Position and do this instead: R.start <= Pos && Pos < R.end (note that LSP ranges are half-open, i.e. the end is excluded).
Even better: define a bool contains(Position Pos) const helper in the Range class and use it here (will abstract away to half-open nature of the range)

186

Let's follow the pattern of decls and macros here. I.e. not return from the function, merely collect all the results.

unittests/clangd/XRefsTests.cpp

59

NIT: remove this class, use IgnoreDiagnostics from Compiler.h instead.

246

Could we also add tests for corner cases: cursor before opening quote, cursor after the closing quote, cursor in the middle of #include token? (we shouldn't navigate anywhere in the middle of the #include token)

[malaperle]

unittests/clangd/XRefsTests.cpp

59

It looks like IgnoreDiagnostics from Compiler.h implements DiagnosticConsumer from Compiler.h, and not DiagnosticsConsumer from ClangdServer.h so that wouldn't work.

[malaperle]

unittests/clangd/XRefsTests.cpp

246

cursor before opening quote, cursor after the closing quote

I assume we don't want to navigate anywhere for these positions? I don't have an opinion.

(we shouldn't navigate anywhere in the middle of the #include token)

It did in CDT and I thought it worked nicely as it made it easier to click on it. You can hold ctrl on the whole line and it underlined it (well except trailing comments). But clients don't handle the spaces nicely (underline between #include and file name) so I thought I'd work on the client first before making the server do it. Anyhow, for now it shouldn't navigate indeed.

[ilya-biryukov]

unittests/clangd/XRefsTests.cpp

59

You're right, I got confused, sorry. Disregard my comment.

246

> cursor before opening quote, cursor after the closing quote
I assume we don't want to navigate anywhere for these positions? I don't have an opinion.

I'd say we should navigate when the cursor is right before the opening quote. For the closing quote I don't have any opinions either :-)

[malaperle]

Fix some NITs, add more tests.

[ilya-biryukov]

Last drop of NITs. After they're fixed, this change is ready to land!

unittests/clangd/XRefsTests.cpp

282

There's a typo, should be HeaderAnnotations

302

NIT: one assertion using EXPECT_THAT will read better and produce more helpful messages in case of failures:

// Create FooHUri to avoid typing URIForFile everywhere.
auto FooHUri = URIForFile{FooH.str()};

EXPECT_THAT(Locations, ElementsAre(Location{FooHUri, HeaderAnnoations.range()}));

328

NIT: Use EXPECT_THAT here too:
EXPECT_THAT(Locations, IsEmpty());

[malaperle]

Use EXPECT_THAT in a few places and clean-ups in tests.

[ilya-biryukov]

LGTM modulo the ASSERT_TRUE nit.

unittests/clangd/XRefsTests.cpp

292

Maybe replace with ASSERT_TRUE?
If Locations has an error, an assertion will fire in debug mode inside Expected that the error was not handled.

[malaperle]

Change some EXPECT_TRUE to ASSERT_TRUE

clangd/ClangdUnit.h

 namespace clangd {
 
 /// A diagnostic with its FixIts.
 struct DiagWithFixIts {
   clangd::Diagnostic Diag;
   llvm::SmallVector<TextEdit, 1> FixIts;
 };
 
+using InclusionLocations = std::vector<std::pair<Range, Path>>;
+
 // Stores Preamble and associated data.
 struct PreambleData {
   PreambleData(PrecompiledPreamble Preamble,
                std::vector<serialization::DeclID> TopLevelDeclIDs,
-               std::vector<DiagWithFixIts> Diags);
+               std::vector<DiagWithFixIts> Diags,
+               InclusionLocations IncLocations);
 
   PrecompiledPreamble Preamble;
   std::vector<serialization::DeclID> TopLevelDeclIDs;
   std::vector<DiagWithFixIts> Diags;
+  InclusionLocations IncLocations;
 };
 
 /// Information required to run clang, e.g. to parse AST or do code completion.
 struct ParseInputs {
   tooling::CompileCommand CompileCommand;
   IntrusiveRefCntPtr<vfs::FileSystem> FS;
   std::string Contents;
 };
 
 /// Stores and provides access to parsed AST.
 class ParsedAST {
 public:
   /// Attempts to run Clang and store parsed AST. If \p Preamble is non-null
   /// it is reused during parsing.
   static llvm::Optional<ParsedAST>
   Build(std::unique_ptr<clang::CompilerInvocation> CI,
         std::shared_ptr<const PreambleData> Preamble,
public:
 
   /// This function returns all top-level decls, including those that come
   /// from Preamble. Decls, coming from Preamble, have to be deserialized, so
   /// this call might be expensive.
   ArrayRef<const Decl *> getTopLevelDecls();
 
   const std::vector<DiagWithFixIts> &getDiagnostics() const;
 
   /// Returns the esitmated size of the AST and the accessory structures, in
   /// bytes. Does not include the size of the preamble.
   std::size_t getUsedBytes() const;
+  const InclusionLocations &getInclusionLocations() const;
 
 private:
   ParsedAST(std::shared_ptr<const PreambleData> Preamble,
             std::unique_ptr<CompilerInstance> Clang,
             std::unique_ptr<FrontendAction> Action,
             std::vector<const Decl *> TopLevelDecls,
-            std::vector<DiagWithFixIts> Diags);
+            std::vector<DiagWithFixIts> Diags, InclusionLocations IncLocations);
 
 private:
   void ensurePreambleDeclsDeserialized();
 
   // In-memory preambles must outlive the AST, it is important that this member
   // goes before Clang and Action.
   std::shared_ptr<const PreambleData> Preamble;
   // We store an "incomplete" FrontendAction (i.e. no EndSourceFile was called
   // on it) and CompilerInstance used to run it. That way we don't have to do
   // complex memory management of all Clang structures on our own. (They are
   // stored in CompilerInstance and cleaned up by
   // FrontendAction.EndSourceFile).
   std::unique_ptr<CompilerInstance> Clang;
   std::unique_ptr<FrontendAction> Action;
 
   // Data, stored after parsing.
   std::vector<DiagWithFixIts> Diags;
   std::vector<const Decl *> TopLevelDecls;
   bool PreambleDeclsDeserialized;
+  InclusionLocations IncLocations;
 };
 
 using ASTParsedCallback = std::function<void(PathRef Path, ParsedAST *)>;
 
 /// Manages resources, required by clangd. Allows to rebuild file with new
 /// contents, and provides AST and Preamble for it.
 class CppFile {
 public:
   CppFile(PathRef FileName, bool StorePreamblesInMemory,
           std::shared_ptr<PCHContainerOperations> PCHs,
           ASTParsedCallback ASTCallback);
private:
   /// Utility class required by clang
   std::shared_ptr<PCHContainerOperations> PCHs;
   /// This is called after the file is parsed. This can be nullptr if there is
   /// no callback.
   ASTParsedCallback ASTCallback;
 };
 
 /// Get the beginning SourceLocation at a specified \p Pos.
 SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos,
                                         const FileEntry *FE);
 
 /// For testing/debugging purposes. Note that this method deserializes all
 /// unserialized Decls, so use with care.
 void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS);
 
 } // namespace clangd
 } // namespace clang
 #endif

clangd/ClangdUnit.cpp

 #include "clang/Sema/Sema.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 
 using namespace clang::clangd;
 using namespace clang;
 
 namespace {
 
 template <class T> std::size_t getUsedBytes(const std::vector<T> &Vec) {
   return Vec.capacity() * sizeof(T);
 }
 
 class DeclTrackingASTConsumer : public ASTConsumer {
 public:
   DeclTrackingASTConsumer(std::vector<const Decl *> &TopLevelDecls)
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
                                                  StringRef InFile) override {
     return llvm::make_unique<DeclTrackingASTConsumer>(/*ref*/ TopLevelDecls);
   }
 
 private:
   std::vector<const Decl *> TopLevelDecls;
 };
 
+// Converts a half-open clang source range to an LSP range.
+// Note that clang also uses closed source ranges, which this can't handle!
+Range toRange(CharSourceRange R, const SourceManager &M) {
+  // Clang is 1-based, LSP uses 0-based indexes.
+  Position Begin;
+  Begin.line = static_cast<int>(M.getSpellingLineNumber(R.getBegin())) - 1;
+  Begin.character =
+      static_cast<int>(M.getSpellingColumnNumber(R.getBegin())) - 1;
+
+  Position End;
+  End.line = static_cast<int>(M.getSpellingLineNumber(R.getEnd())) - 1;
+  End.character = static_cast<int>(M.getSpellingColumnNumber(R.getEnd())) - 1;
+
+  return {Begin, End};
+}
+
+class InclusionLocationsCollector : public PPCallbacks {
+public:
+  InclusionLocationsCollector(SourceManager &SourceMgr,
+                              InclusionLocations &IncLocations)
+      : SourceMgr(SourceMgr), IncLocations(IncLocations) {}
+
+  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+                          StringRef FileName, bool IsAngled,
+                          CharSourceRange FilenameRange, const FileEntry *File,
+                          StringRef SearchPath, StringRef RelativePath,
+                          const Module *Imported) override {
+    auto SR = FilenameRange.getAsRange();
+    if (SR.isInvalid() || !File || File->tryGetRealPathName().empty())
+      return;
+
+    if (SourceMgr.isInMainFile(SR.getBegin())) {
+      // Only inclusion directives in the main file make sense. The user cannot
+      // select directives not in the main file.
+      IncLocations.emplace_back(toRange(FilenameRange, SourceMgr),
+                                File->tryGetRealPathName());
+    }
+  }
+
+private:
+  SourceManager &SourceMgr;
+  InclusionLocations &IncLocations;
+};
+
 class CppFilePreambleCallbacks : public PreambleCallbacks {
 public:
   std::vector<serialization::DeclID> takeTopLevelDeclIDs() {
     return std::move(TopLevelDeclIDs);
   }
 
+  InclusionLocations takeInclusionLocations() {
+    return std::move(IncLocations);
+  }
+
   void AfterPCHEmitted(ASTWriter &Writer) override {
     TopLevelDeclIDs.reserve(TopLevelDecls.size());
     for (Decl *D : TopLevelDecls) {
       // Invalid top-level decls may not have been serialized.
       if (D->isInvalidDecl())
         continue;
       TopLevelDeclIDs.push_back(Writer.getDeclID(D));
     }
   }
 
   void HandleTopLevelDecl(DeclGroupRef DG) override {
     for (Decl *D : DG) {
       if (isa<ObjCMethodDecl>(D))
         continue;
       TopLevelDecls.push_back(D);
     }
   }
 
+  void BeforeExecute(CompilerInstance &CI) override {
+    SourceMgr = &CI.getSourceManager();
+  }
+
+  std::unique_ptr<PPCallbacks> createPPCallbacks() override {
+    assert(SourceMgr && "SourceMgr must be set at this point");
+    return llvm::make_unique<InclusionLocationsCollector>(*SourceMgr,
+                                                          IncLocations);
+  }
+
 private:
   std::vector<Decl *> TopLevelDecls;
   std::vector<serialization::DeclID> TopLevelDeclIDs;
+  InclusionLocations IncLocations;
+  SourceManager *SourceMgr = nullptr;
 };
 
 /// Convert from clang diagnostic level to LSP severity.
 static int getSeverity(DiagnosticsEngine::Level L) {
   switch (L) {
   case DiagnosticsEngine::Remark:
     return 4;
   case DiagnosticsEngine::Note:
     return 3;
   case DiagnosticsEngine::Warning:
     return 2;
   case DiagnosticsEngine::Fatal:
   case DiagnosticsEngine::Error:
     return 1;
   case DiagnosticsEngine::Ignored:
     return 0;
   }
   llvm_unreachable("Unknown diagnostic level!");
 }
 
 // Checks whether a location is within a half-open range.
 // Note that clang also uses closed source ranges, which this can't handle!
 bool locationInRange(SourceLocation L, CharSourceRange R,
                      const SourceManager &M) {
   assert(R.isCharRange());
   if (!R.isValid() || M.getFileID(R.getBegin()) != M.getFileID(R.getEnd()) ||
       M.getFileID(R.getBegin()) != M.getFileID(L))
     return false;
   return L != R.getEnd() && M.isPointWithin(L, R.getBegin(), R.getEnd());
 }
 
-// Converts a half-open clang source range to an LSP range.
-// Note that clang also uses closed source ranges, which this can't handle!
-Range toRange(CharSourceRange R, const SourceManager &M) {
-  // Clang is 1-based, LSP uses 0-based indexes.
-  Position Begin;
-  Begin.line = static_cast<int>(M.getSpellingLineNumber(R.getBegin())) - 1;
-  Begin.character =
-      static_cast<int>(M.getSpellingColumnNumber(R.getBegin())) - 1;
-
-  Position End;
-  End.line = static_cast<int>(M.getSpellingLineNumber(R.getEnd())) - 1;
-  End.character = static_cast<int>(M.getSpellingColumnNumber(R.getEnd())) - 1;
-
-  return {Begin, End};
-}
-
 // Clang diags have a location (shown as ^) and 0 or more ranges (~~~~).
 // LSP needs a single range.
 Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
   auto &M = D.getSourceManager();
   auto Loc = M.getFileLoc(D.getLocation());
   // Accept the first range that contains the location.
   for (const auto &CR : D.getRanges()) {
     auto R = Lexer::makeFileCharRange(CR, M, L);
     if (locationInRange(Loc, R, M))
       return toRange(R, M);
   }
   // The range may be given as a fixit hint instead.
   for (const auto &F : D.getFixItHints()) {
     auto R = Lexer::makeFileCharRange(F.RemoveRange, M, L);
     if (locationInRange(Loc, R, M))
       return toRange(R, M);
   }
   // If no suitable range is found, just use the token at the location.
   auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
   if (!R.isValid()) // Fall back to location only, let the editor deal with it.
     R = CharSourceRange::getCharRange(Loc);
   return toRange(R, M);
 }
 
 TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
                     const LangOptions &L) {
   TextEdit Result;
   Result.range = toRange(Lexer::makeFileCharRange(FixIt.RemoveRange, M, L), M);
if (LangOpts)
         Output.push_back(std::move(*D));
   }
 
 private:
   std::vector<DiagWithFixIts> &Output;
   llvm::Optional<LangOptions> LangOpts;
 };
 
 } // namespace
 
 void clangd::dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) {
   AST.getASTContext().getTranslationUnitDecl()->dump(OS, true);
 }
 
 llvm::Optional<ParsedAST>
 ParsedAST::Build(std::unique_ptr<clang::CompilerInvocation> CI,
                  std::shared_ptr<const PreambleData> Preamble,
                  std::unique_ptr<llvm::MemoryBuffer> Buffer,
                  std::shared_ptr<PCHContainerOperations> PCHs,
                  IntrusiveRefCntPtr<vfs::FileSystem> VFS) {
   std::vector<DiagWithFixIts> ASTDiags;
   StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags);
 
   const PrecompiledPreamble *PreamblePCH =
       Preamble ? &Preamble->Preamble : nullptr;
   auto Clang = prepareCompilerInstance(
       std::move(CI), PreamblePCH, std::move(Buffer), std::move(PCHs),
       std::move(VFS), /*ref*/ UnitDiagsConsumer);
   if (!Clang)
     return llvm::None;
 
   // Recover resources if we crash before exiting this method.
   llvm::CrashRecoveryContextCleanupRegistrar<CompilerInstance> CICleanup(
       Clang.get());
 
   auto Action = llvm::make_unique<ClangdFrontendAction>();
   const FrontendInputFile &MainInput = Clang->getFrontendOpts().Inputs[0];
   if (!Action->BeginSourceFile(*Clang, MainInput)) {
     log("BeginSourceFile() failed when building AST for " +
         MainInput.getFile());
     return llvm::None;
   }
+
+  InclusionLocations IncLocations;
+  // Copy over the includes from the preamble, then combine with the
+  // non-preamble includes below.
+  if (Preamble)
+    IncLocations = Preamble->IncLocations;
+
+  Clang->getPreprocessor().addPPCallbacks(
+      llvm::make_unique<InclusionLocationsCollector>(Clang->getSourceManager(),
+                                                     IncLocations));
+
   if (!Action->Execute())
     log("Execute() failed when building AST for " + MainInput.getFile());
 
   // UnitDiagsConsumer is local, we can not store it in CompilerInstance that
   // has a longer lifetime.
   Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
 
   std::vector<const Decl *> ParsedDecls = Action->takeTopLevelDecls();
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
-                   std::move(ParsedDecls), std::move(ASTDiags));
+                   std::move(ParsedDecls), std::move(ASTDiags),
+                   std::move(IncLocations));
 }
 
 namespace {
 
 SourceLocation getMacroArgExpandedLocation(const SourceManager &Mgr,
                                            const FileEntry *FE, Position Pos) {
   SourceLocation InputLoc =
       Mgr.translateFileLineCol(FE, Pos.line + 1, Pos.character + 1);
   return Mgr.getMacroArgExpandedLocation(InputLoc);
 }
 
 } // namespace
 
 void ParsedAST::ensurePreambleDeclsDeserialized() {
   if (PreambleDeclsDeserialized || !Preamble)
     return;
 
   std::vector<const Decl *> Resolved;
   Resolved.reserve(Preamble->TopLevelDeclIDs.size());
 
   ExternalASTSource &Source = *getASTContext().getExternalSource();
   for (serialization::DeclID TopLevelDecl : Preamble->TopLevelDeclIDs) {
     // Resolve the declaration ID to an actual declaration, possibly
     // deserializing the declaration in the process.
     if (Decl *D = Source.GetExternalDecl(TopLevelDecl))
       Resolved.push_back(D);
   }
 

 std::size_t ParsedAST::getUsedBytes() const {
   auto &AST = getASTContext();
   // FIXME(ibiryukov): we do not account for the dynamically allocated part of
   // SmallVector<FixIt> inside each Diag.
   return AST.getASTAllocatedMemory() + AST.getSideTableAllocatedMemory() +
          ::getUsedBytes(TopLevelDecls) + ::getUsedBytes(Diags);
 }
 
+const InclusionLocations &ParsedAST::getInclusionLocations() const {
+  return IncLocations;
+}
+
 PreambleData::PreambleData(PrecompiledPreamble Preamble,
                            std::vector<serialization::DeclID> TopLevelDeclIDs,
-                           std::vector<DiagWithFixIts> Diags)
+                           std::vector<DiagWithFixIts> Diags,
+                           InclusionLocations IncLocations)
     : Preamble(std::move(Preamble)),
-      TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)) {}
+      TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)),
+      IncLocations(std::move(IncLocations)) {}
 
 ParsedAST::ParsedAST(std::shared_ptr<const PreambleData> Preamble,
                      std::unique_ptr<CompilerInstance> Clang,
                      std::unique_ptr<FrontendAction> Action,
                      std::vector<const Decl *> TopLevelDecls,
-                     std::vector<DiagWithFixIts> Diags)
+                     std::vector<DiagWithFixIts> Diags,
+                     InclusionLocations IncLocations)
     : Preamble(std::move(Preamble)), Clang(std::move(Clang)),
       Action(std::move(Action)), Diags(std::move(Diags)),
-      TopLevelDecls(std::move(TopLevelDecls)),
-      PreambleDeclsDeserialized(false) {
+      TopLevelDecls(std::move(TopLevelDecls)), PreambleDeclsDeserialized(false),
+      IncLocations(std::move(IncLocations)) {
   assert(this->Clang);
   assert(this->Action);
 }
 
 CppFile::CppFile(PathRef FileName, bool StorePreamblesInMemory,
                  std::shared_ptr<PCHContainerOperations> PCHs,
                  ASTParsedCallback ASTCallback)
     : FileName(FileName), StorePreamblesInMemory(StorePreamblesInMemory),
std::unique_ptr<CompilerInvocation> CI;
     CI = createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine,
                                          Inputs.FS);
     if (!CI) {
       log("Could not build CompilerInvocation for file " + FileName);
       AST = llvm::None;
       Preamble = nullptr;
       return llvm::None;
     }
     // createInvocationFromCommandLine sets DisableFree.
     CI->getFrontendOpts().DisableFree = false;
   }
 
   std::unique_ptr<llvm::MemoryBuffer> ContentsBuffer =
       llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, FileName);
 
   // Compute updated Preamble.
   std::shared_ptr<const PreambleData> NewPreamble =
       rebuildPreamble(*CI, Inputs.FS, *ContentsBuffer);
 
   // Remove current AST to avoid wasting memory.
   AST = llvm::None;
   // Compute updated AST.
CppFile::rebuildPreamble(CompilerInvocation &CI,
 
   if (BuiltPreamble) {
     log("Built preamble of size " + Twine(BuiltPreamble->getSize()) +
         " for file " + Twine(FileName));
 
     return std::make_shared<PreambleData>(
         std::move(*BuiltPreamble),
         SerializedDeclsCollector.takeTopLevelDeclIDs(),
-        std::move(PreambleDiags));
+        std::move(PreambleDiags),
+        SerializedDeclsCollector.takeInclusionLocations());
   } else {
     log("Could not build a preamble for file " + Twine(FileName));
     return nullptr;
   }
 }
 
 SourceLocation clangd::getBeginningOfIdentifier(ParsedAST &Unit,
                                                 const Position &Pos,

clangd/Protocol.h

struct Position {
   friend bool operator==(const Position &LHS, const Position &RHS) {
     return std::tie(LHS.line, LHS.character) ==
            std::tie(RHS.line, RHS.character);
   }
   friend bool operator<(const Position &LHS, const Position &RHS) {
     return std::tie(LHS.line, LHS.character) <
            std::tie(RHS.line, RHS.character);
   }
+  friend bool operator<=(const Position &LHS, const Position &RHS) {
+    return std::tie(LHS.line, LHS.character) <=
+           std::tie(RHS.line, RHS.character);
+  }
 };
 bool fromJSON(const json::Expr &, Position &);
 json::Expr toJSON(const Position &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Position &);
 
 struct Range {
   /// The range's start position.
   Position start;
 
   /// The range's end position.
   Position end;
 
   friend bool operator==(const Range &LHS, const Range &RHS) {
     return std::tie(LHS.start, LHS.end) == std::tie(RHS.start, RHS.end);
   }
   friend bool operator<(const Range &LHS, const Range &RHS) {
     return std::tie(LHS.start, LHS.end) < std::tie(RHS.start, RHS.end);
   }
+
+  bool contains(Position Pos) const { return start <= Pos && Pos < end; }
 };
 bool fromJSON(const json::Expr &, Range &);
 json::Expr toJSON(const Range &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Range &);
 
 struct Location {
   /// The text document's URI.
   URIForFile uri;

clangd/SourceCode.h

 //===--- SourceCode.h - Manipulating source code as strings -----*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Various code that examines C++ source code without using heavy AST machinery
 // (and often not even the lexer). To be used sparingly!
 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
 #include "Protocol.h"
+#include "clang/Basic/SourceLocation.h"
 
 namespace clang {
+class SourceManager;
+
 namespace clangd {
 
 /// Turn a [line, column] pair into an offset in Code.
 size_t positionToOffset(llvm::StringRef Code, Position P);
 
 /// Turn an offset in Code into a [line, column] pair.
 Position offsetToPosition(llvm::StringRef Code, size_t Offset);
 
+/// Turn a SourceLocation into a [line, column] pair.
+Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc);
+
 } // namespace clangd
 } // namespace clang
 #endif

clangd/SourceCode.cpp

 //===--- SourceCode.h - Manipulating source code as strings -----*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 #include "SourceCode.h"
 
+#include "clang/Basic/SourceManager.h"
+
 namespace clang {
 namespace clangd {
 using namespace llvm;
 
 size_t positionToOffset(StringRef Code, Position P) {
   if (P.line < 0)
     return 0;
   size_t StartOfLine = 0;
Position offsetToPosition(StringRef Code, size_t Offset) {
   size_t StartOfLine = (PrevNL == StringRef::npos) ? 0 : (PrevNL + 1);
   // FIXME: officially character counts UTF-16 code units, not UTF-8 bytes!
   Position Pos;
   Pos.line = Lines;
   Pos.character = static_cast<int>(Offset - StartOfLine);
   return Pos;
 }
 
+Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc) {
+  Position P;
+  P.line = static_cast<int>(SM.getSpellingLineNumber(Loc)) - 1;
+  P.character = static_cast<int>(SM.getSpellingColumnNumber(Loc)) - 1;
+  return P;
+}
+
 } // namespace clangd
 } // namespace clang
-

clangd/XRefs.cpp

 //===--- XRefs.cpp ----------------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 #include "XRefs.h"
 #include "Logger.h"
+#include "SourceCode.h"
 #include "URI.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexingAction.h"
 #include "llvm/Support/Path.h"
 namespace clang {
 namespace clangd {
 using namespace llvm;
 namespace {
std::vector<const Decl *> takeDecls() {
     // Don't keep the same declaration multiple times.
     // This can happen when nodes in the AST are visited twice.
     std::sort(Decls.begin(), Decls.end());
     auto Last = std::unique(Decls.begin(), Decls.end());
     Decls.erase(Last, Decls.end());
     return std::move(Decls);
   }
 
   std::vector<const MacroInfo *> takeMacroInfos() {
     // Don't keep the same Macro info multiple times.
     std::sort(MacroInfos.begin(), MacroInfos.end());
     auto Last = std::unique(MacroInfos.begin(), MacroInfos.end());
     MacroInfos.erase(Last, MacroInfos.end());
     return std::move(MacroInfos);
   }
 
   bool
getDeclarationLocation(ParsedAST &AST, const SourceRange &ValSourceRange) {
   SourceLocation LocStart = ValSourceRange.getBegin();
 
   const FileEntry *F =
       SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart));
   if (!F)
     return llvm::None;
   SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(), 0,
                                                      SourceMgr, LangOpts);
-  Position Begin;
-  Begin.line = SourceMgr.getSpellingLineNumber(LocStart) - 1;
-  Begin.character = SourceMgr.getSpellingColumnNumber(LocStart) - 1;
-  Position End;
-  End.line = SourceMgr.getSpellingLineNumber(LocEnd) - 1;
-  End.character = SourceMgr.getSpellingColumnNumber(LocEnd) - 1;
+  Position Begin = sourceLocToPosition(SourceMgr, LocStart);
+  Position End = sourceLocToPosition(SourceMgr, LocEnd);
   Range R = {Begin, End};
   Location L;
 
   SmallString<64> FilePath = F->tryGetRealPathName();
   if (FilePath.empty())
     FilePath = F->getName();
   if (!llvm::sys::path::is_absolute(FilePath)) {
     if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos) {
   index::IndexingOptions IndexOpts;
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
 
   indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
                      DeclMacrosFinder, IndexOpts);
 
   std::vector<const Decl *> Decls = DeclMacrosFinder->takeDecls();
   std::vector<const MacroInfo *> MacroInfos =
       DeclMacrosFinder->takeMacroInfos();
   std::vector<Location> Result;
 
   for (auto Item : Decls) {
     auto L = getDeclarationLocation(AST, Item->getSourceRange());
     if (L)
       Result.push_back(*L);
   }
 
   for (auto Item : MacroInfos) {
     SourceRange SR(Item->getDefinitionLoc(), Item->getDefinitionEndLoc());
     auto L = getDeclarationLocation(AST, SR);
     if (L)
       Result.push_back(*L);
   }
 
+  /// Process targets for paths inside #include directive.
+  for (auto &IncludeLoc : AST.getInclusionLocations()) {
+    Range R = IncludeLoc.first;
+    Position Pos = sourceLocToPosition(SourceMgr, SourceLocationBeg);
+
+    if (R.contains(Pos))
+      Result.push_back(Location{URIForFile{IncludeLoc.second}, {}});
+  }
+
   return Result;
 }
 
 namespace {
 
 /// Finds document highlights that a given list of declarations refers to.
 class DocumentHighlightsFinder : public index::IndexDataConsumer {
   std::vector<const Decl *> &Decls;
handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
     DocumentHighlights.push_back(getDocumentHighlight(SR, Kind));
     return true;
   }
 
 private:
   DocumentHighlight getDocumentHighlight(SourceRange SR,
                                          DocumentHighlightKind Kind) {
     const SourceManager &SourceMgr = AST.getSourceManager();
-    SourceLocation LocStart = SR.getBegin();
-    Position Begin;
-    Begin.line = SourceMgr.getSpellingLineNumber(LocStart) - 1;
-    Begin.character = SourceMgr.getSpellingColumnNumber(LocStart) - 1;
-    Position End;
-    End.line = SourceMgr.getSpellingLineNumber(SR.getEnd()) - 1;
-    End.character = SourceMgr.getSpellingColumnNumber(SR.getEnd()) - 1;
+    Position Begin = sourceLocToPosition(SourceMgr, SR.getBegin());
+    Position End = sourceLocToPosition(SourceMgr, SR.getEnd());
     Range R = {Begin, End};
     DocumentHighlight DH;
     DH.range = R;
     DH.kind = Kind;
     return DH;
   }
 };
 

unittests/clangd/XRefsTests.cpp

ParsedAST build(StringRef Code) {
   auto Buf = MemoryBuffer::getMemBuffer(Code);
   auto AST = ParsedAST::Build(std::move(CI), nullptr, std::move(Buf),
                               std::make_shared<PCHContainerOperations>(),
                               vfs::getRealFileSystem());
   assert(AST.hasValue());
   return std::move(*AST);
 }
 
 // Extracts ranges from an annotated example, and constructs a matcher for a
 // highlight set. Ranges should be named $read/$write as appropriate.
 Matcher<const std::vector<DocumentHighlight> &>
 HighlightsFrom(const Annotations &Test) {
   std::vector<DocumentHighlight> Expected;
   auto Add = [&](const Range &R, DocumentHighlightKind K) {
     Expected.emplace_back();
     Expected.back().range = R;
     Expected.back().kind = K;

 [[int foo]];
 int baz = f^oo;
 )cpp");
 
   IgnoreDiagnostics DiagConsumer;
   MockCompilationDatabase CDB(/*UseRelPaths=*/true);
   MockFSProvider FS;
   ClangdServer Server(CDB, DiagConsumer, FS, /*AsyncThreadsCount=*/0,
                       /*StorePreambleInMemory=*/true);
 
   auto FooCpp = getVirtualTestFilePath("foo.cpp");
   FS.Files[FooCpp] = "";
 
   Server.addDocument(FooCpp, SourceAnnotations.code());
   auto Locations = Server.findDefinitions(FooCpp, SourceAnnotations.point());
   EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
 
   EXPECT_THAT(Locations->Value,
               ElementsAre(Location{URIForFile{FooCpp.str()},
                                    SourceAnnotations.range()}));
 }
 
+TEST(GoToInclude, All) {
+  MockFSProvider FS;
+  IgnoreDiagnostics DiagConsumer;
+  MockCompilationDatabase CDB;
+
+  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
+                      /*StorePreamblesInMemory=*/true);
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  const char *SourceContents = R"cpp(
+  #include ^"$2^foo.h$3^"
+  #include "$4^invalid.h"
+  int b = a;
+  // test
+  int foo;
+  #in$5^clude "$6^foo.h"$7^
+  )cpp";
+  Annotations SourceAnnoations(SourceContents);
+  FS.Files[FooCpp] = SourceAnnoations.code();
+  auto FooH = getVirtualTestFilePath("foo.h");
+
+  const char *HeaderContents = R"cpp([[]]int a;)cpp";
+  Annotations HeaderAnnoations(HeaderContents);
+  FS.Files[FooH] = HeaderAnnoations.code();
+
+  Server.addDocument(FooH, HeaderAnnoations.code());
+  Server.addDocument(FooCpp, SourceAnnoations.code());
+
+  // Test include in preamble.
+  auto ExpectedLocations =
+      Server.findDefinitions(FooCpp, SourceAnnoations.point());
+  ASSERT_TRUE(!!ExpectedLocations);
+  std::vector<Location> Locations = ExpectedLocations->Value;
+  EXPECT_TRUE(!Locations.empty());
+  ASSERT_EQ(Locations[0].uri.file, FooH);
+  ASSERT_EQ(Locations[0].range, HeaderAnnoations.range());
+
+  // Test include in preamble, last char.
+  ExpectedLocations =
+      Server.findDefinitions(FooCpp, SourceAnnoations.point("2"));
+  ASSERT_TRUE(!!ExpectedLocations);
+  Locations = ExpectedLocations->Value;
+  EXPECT_TRUE(!Locations.empty());
+  ASSERT_EQ(Locations[0].uri.file, FooH);
+  ASSERT_EQ(Locations[0].range, HeaderAnnoations.range());
+
+  ExpectedLocations =
+      Server.findDefinitions(FooCpp, SourceAnnoations.point("3"));
+  ASSERT_TRUE(!!ExpectedLocations);
+  Locations = ExpectedLocations->Value;
+  EXPECT_TRUE(!Locations.empty());
+  ASSERT_EQ(Locations[0].uri.file, FooH);
+  ASSERT_EQ(Locations[0].range, HeaderAnnoations.range());
+
+  // Test include outside of preamble.
+  ExpectedLocations =
+      Server.findDefinitions(FooCpp, SourceAnnoations.point("6"));
+  ASSERT_TRUE(!!ExpectedLocations);
+  Locations = ExpectedLocations->Value;
+  EXPECT_TRUE(!Locations.empty());
+  ASSERT_EQ(Locations[0].uri.file, FooH);
+  ASSERT_EQ(Locations[0].range, HeaderAnnoations.range());
+
+  // Test a few positions that do not result in Locations.
+  ExpectedLocations =
+      Server.findDefinitions(FooCpp, SourceAnnoations.point("4"));
+  ASSERT_TRUE(!!ExpectedLocations);
+  Locations = ExpectedLocations->Value;
+  EXPECT_TRUE(Locations.empty());
+
+  ExpectedLocations =
+      Server.findDefinitions(FooCpp, SourceAnnoations.point("5"));
+  ASSERT_TRUE(!!ExpectedLocations);
+  Locations = ExpectedLocations->Value;
+  EXPECT_TRUE(Locations.empty());
+
+  ExpectedLocations =
+      Server.findDefinitions(FooCpp, SourceAnnoations.point("7"));
+  ASSERT_TRUE(!!ExpectedLocations);
+  Locations = ExpectedLocations->Value;
+  EXPECT_TRUE(Locations.empty());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang