[PATCH] D128457: [clangd] Add new IncludeType to IncludeHeaderWithReferences
Kadir Cetinkaya via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 6 00:25:37 PST 2022
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
More information about the cfe-commits
mailing list