[PATCH] D143274: [clangd] Remove the direct use of StdSymbolMapping.inc usage.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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