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

William Enright via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 19 14:15:46 PST 2017

Nebiroth marked 8 inline comments as done.
Nebiroth added inline comments.

Comment at: clangd/ClangdUnit.cpp:85
+    if (SourceMgr.getMainFileID() == SourceMgr.getFileID(FilenameRange.getAsRange().getBegin()) && SourceMgr.isInMainFile(FilenameRange.getAsRange().getBegin())) {
+      // Only inclusion directives in the main file make sense. The user cannot
ilya-biryukov wrote:
> ilya-biryukov wrote:
> > `clang-format` please
> Do we need both checks? Doesn't `SourceMgr.isInMainFile` handles all the cases?
This check was here to prevent an issue that appeared previously. I don't think this check is needed anymore.

Comment at: clangd/ClangdUnit.cpp:147
+  IncludeReferenceMap &IRM;
+  std::vector<std::pair<SourceRange, std::string>> TempPreambleIncludeLocations;
ilya-biryukov wrote:
> We should have `SourceMgr` at all the proper places now, let's store `IncludeReferenceMap` directly
The idea was to pass a single map around to fill with every reference instead of having separate maps being merged together.

Comment at: clangd/ClangdUnit.cpp:281
+                 IntrusiveRefCntPtr<vfs::FileSystem> VFS,
+                 IncludeReferenceMap IRM) {
ilya-biryukov wrote:
> What reference does this `IncludeReferenceMap` contain? Includes from the preamble? 
This should contain every reference available for one file.

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list