[PATCH] D143274: [clangd] Remove the direct use of StdSymbolMapping.inc usage.
Kadir Cetinkaya via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 6 02:35:33 PST 2023
kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:711
+ return "<utility>";
+ // There are multiple headers for size_t, pick one.
+ if (QName == "std::size_t")
----------------
i think the comment is misleading. as if we had some alternatives in the tooling::stdlib, it would already pick one for us. the issue is we don't have it in the mapping at all.
================
Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:717
+ Scope, Name,
+ L.CPlusPlus ? tooling::stdlib::Lang::CXX : tooling::stdlib::Lang::C))
+ return StdSym->header().name();
----------------
this is a slight behaviour change. we'd actually return `""` whenever the language isn't CPlusPlus or C11. could you retain that behaviour?
================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:840
QName.append(S->Name);
- IncludeHeader = Opts.Includes->mapSymbol(QName);
+ IncludeHeader = Opts.Includes->mapSymbol(QName, ASTCtx->getLangOpts());
if (!IncludeHeader.empty()) {
----------------
nit: rather than creating a new copy of the symbol name everytime, i'd also update the `mapSymbol` interface to take in `StringRef Scope, StringRef Name` instead.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143274/new/
https://reviews.llvm.org/D143274
More information about the cfe-commits
mailing list