[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 18 07:59:56 PST 2019
sammccall added inline comments.
================
Comment at: clangd/index/CanonicalIncludes.cpp:123
static const std::vector<std::pair<const char *, const char *>>
SystemHeaderMap = {
----------------
hokein wrote:
> ioeric wrote:
> > Can we remove the suffix header mapping now? Is it for the `std::chrono::` symbols? What are the reasons not to include them in this patch?
> Ideally, we could remove it once we get a perfect symbol mapping but I'd prefer to keep it (it serves as a fallback when we don't find symbols in the symbol mapping), so that we could add new symbol mapping increasingly without changing the current behavior.
>
> Removing it should be done carefully without introducing regression, and is outside the scope of this patch.
We do need fallback heuristics. We should conisder cases:
- for known `std::` symbols mapped to one system header, we don't need a fallback
- for known `std::` mapped to multiple system headers (`std::move`), we need some fallback. There are probably few enough of these that it doesn't justify listing *all* system header paths.
- for known `std::` symbols with a primary template in one file and specializations/overloads in another (swap?) always inserting the primary seems fine
- For "random" unknown symbols from system header `foo/bits/_bar.h`, we can not insert, correct to `<bar>`, or use the full path name. I don't have a strong preference here, maybe we should do what's easiest.
- for c standard library (`::printf` --> `<stdio.h>` etc) we probably want mappings just like in this patch, I think cppreference has them.
- other "standardish" headers (POSIX etc) - hmm, unclear.
Thinking about all these cases, I'm thinking the nicest solution would be having the symbol -> header mapping, having a small (symbol,header) -> header mapping **only** for ambiguous cases, and not inserting "system" headers that aren't in the main mapping at all. Then we can improve coverage of posix, windows etc headers over time.
Question is, how can we recognize "system" headers?
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58345/new/
https://reviews.llvm.org/D58345
More information about the cfe-commits
mailing list