[PATCH] D127184: [clangd] Add <bits/ranges_algo.h> to header map

Florian Albrechtskirchinger via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 22 02:17:02 PDT 2022

falbrechtskirchinger added a comment.

In D127184#3588226 <https://reviews.llvm.org/D127184#3588226>, @nridge wrote:

> In D127184#3577165 <https://reviews.llvm.org/D127184#3577165>, @falbrechtskirchinger wrote:
>> bits/mofunc_impl.h
> I see this included from `bits/move_only_function.h`, so I think `<functional>` would make sense for it.

Right. I somehow erroneously concluded that `bits/mofunc_impl.h` was replaced by `bits/move_only_function.h`.

>> bits/new_allocator.h
> I see this included from `<experimental/memory_resource>`, we could add that. (There are a few more implementation headers in `experimental/bits` which are included by standard headers in `<experimental/...>` that we could consider adding.)

I did see that and excluded `<experimental/...` initially. I can add those as well.

>> bits/specfun.h
> I see this included from `<cmath>`.

I missed the C headers in general because I didn't realize that they're kept in a separate directory in the libstdc++ source tree. The difference between source and install tree has bitten me a few times.

>> I've seen `*.tcc` files being mapped and have identified the following missing files:
>> Should these be added as well?
> I think we can skip these as they only contain definitions, and code completion should choose the file containing the declaration.

Then maybe we should just remove all `*.tcc` files instead? I may do so in a separate commit that can be discarded.

About some seemingly random mappings. There are more candidates. I'll handle those in a separate commit for easy review.

In D127184#3596339 <https://reviews.llvm.org/D127184#3596339>, @sammccall wrote:

> Apologies for not getting to this before vacation, and thanks Nathan for looking at this. (I'll leave for you to stamp)
> This is fine with me as-is, but I think this mapping shouldn't be our *preferred* way to solve this problem, and should eventually go away.
> We also have a mapping of **symbol** names to headers (StdSymbols.inc). This is obviously more portable, both in the sense that it works when editing using e.g. MS STL etc, and that the results don't reflect quirks of the stdlib you're using.
> The reason this mapping fails for `std::ranges::transform` is that the mapping file was extracted from an old C++17 cppreference dump. The cppreference format has changed so to run it on a newer dump it'd need some updates.

With some additional pointers, I'd be happy to look into that next.

One more thing. What about extension headers (`<ext/...>`)?
The current mappings are both very incomplete and also very wrong, or so I'd argue.
For example, `"ext/new_allocator.h"` maps to `<string>` but should map to itself. It's not an internal header but a GNU extension to the C++ standard library.
I've identified and mapped about 50 headers in that category, but omitted the subdirectory `<ext/pb_ds/...>`.

  $ find ext/pb_ds -type f | wc -l

That's a bit much for now.

I'll finish everything up as described within about a week.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list