[PATCH] D143274: [clangd] Remove the direct use of StdSymbolMapping.inc usage.
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 6 05:18:06 PST 2023
hokein marked an inline comment as done.
hokein added a comment.
In D143274#4106449 <https://reviews.llvm.org/D143274#4106449>, @kadircet wrote:
> thanks a lot. since this is the last (and only) upstream user of the raw mappings. can you also move them into `clang/lib/Tooling/Inclusions/Stdlib/` as part of this patch?
As discussed, I will address this in a followup patch.
================
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")
----------------
kadircet wrote:
> 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.
yeah, this comment was moved from the old version. I removed it as it is covered by the above FIXME.
================
Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.h:43
+ /// Returns the overridden include for a qualified symbol with, or "".
+ llvm::StringRef mapSymbol(llvm::StringRef /*std::*/ Scope,
+ llvm::StringRef /*vector*/ Name,
----------------
kadircet wrote:
> these parameter comments are a little bit unconventional, maybe just mention that `Scope` should have trailing colons (e.g. std::) and name shouldn't have any (e.g. vector) ? also mention that `it should be empty ("") for global namespace`?
Removed the parameter comments, and refined the doc comment of the function.
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