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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
