[PATCH] D128457: [clangd] Add new IncludeType to IncludeHeaderWithReferences

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

it feels like rebasing went wrong. changes from 2 unrelated patches seem to be part of this one now. can you make sure this patch only contains the delta for symbolcollector/clangd pieces?



================
Comment at: clang-tools-extra/clangd/Headers.h:50
+  /// The header to include.
+  llvm::StringRef Header;
+  /// The include directive to use, e.g. #import or #include.
----------------
let's mention that this is either a URI or verbatim include in the comments


================
Comment at: clang-tools-extra/clangd/Headers.h:54
 
+struct HeaderInclude {
+  /// The header to include.
----------------
let's rename this to `SymbolInclude` as it looks too similar to `HeaderFile` right now.

also adding a comment like: `A header and directives as stored in a Symbol.`


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:869
+          Directives |= Symbol::Include;
+        if ((CollectDirectives & Symbol::Import) != 0) {
+          auto [It, Inserted] = FileToContainsImportsOrObjC.try_emplace(FID);
----------------
can we add a comment like `Only allow #import for symbols from objc-like files.`


================
Comment at: clang/lib/Tooling/Inclusions/HeaderAnalysis.cpp:76
+      // need to check for anything but the main-file.
+      (!const_cast<SourceManager &>(SM).isMainFile(*FE) ||
+       !codeContainsImports(getFileContents(FE, SM))))
----------------
`SM.getFileEntryForID(SM.getMainFileID()) == FE`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128457/new/

https://reviews.llvm.org/D128457