[PATCH] D38639: [clangd] #include statements support for Open definition

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 9 07:16:48 PST 2017

ilya-biryukov added inline comments.

Comment at: clangd/ClangdServer.cpp:368
   std::vector<Location> Result;
-  Resources->getAST().get()->runUnderLock([Pos, &Result, this](ParsedAST *AST) {
-    if (!AST)
-      return;
-    Result = clangd::findDefinitions(*AST, Pos, Logger);
-  });
+  std::unordered_map<Range, Path, RangeHash> IncludeLocationMap;
+  std::vector<std::pair<SourceRange, std::string>> DataVector;
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);

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

Comment at: clangd/ClangdUnit.cpp:33
 #include <chrono>
+#include <iostream>
We probably don't need it this include

Comment at: clangd/ClangdUnit.cpp:38
+class DelegatingPPCallbacks : public PPCallbacks {
What's the purpose of this class?

Comment at: clangd/ClangdUnit.cpp:131
+  unsigned NumSlocs = SM.local_sloc_entry_size();
+  SmallVector<SourceLocation, 10> InclusionStack;
Please also use `PPCallbacks` interface instead of the `SourceManager` (should be possible to hook it up somewhere in `ParsedAST::Build`).


More information about the cfe-commits mailing list