[PATCH] D127184: [clangd] Add to header map

[Florian Albrechtskirchinger via Phabricator via cfe-commits]

falbrechtskirchinger added a comment.

In D127184#3572731 , @nridge wrote:

> This change looks fine to me.
>
> I wonder though if we should be a bit more systematic about it, and try to 
> cover other newly added libstdc++ implementation headers?

Sure thing.

> (There are other new ones in gcc 11, and some in older versions that we've 
> missed, but I think handling a single gcc version would be a good scope for 
> this patch.)

I don't mind doing a systematic review including older and newer versions. Give 
me a few days to find some spare time to do it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127184/new/

https://reviews.llvm.org/D127184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127184: [clangd] Add to header map

[Florian Albrechtskirchinger via Phabricator via cfe-commits]

falbrechtskirchinger added a comment.

Here's the preliminary list of mappings I've identified:

  "bits/align.h", ""
  "bits/atomic_wait.h", ""
  "bits/atomic_timed_wait.h", ""
  "bits/boost_concept_check.h", ""
  "bits/charconv.h", ""
  "bits/chrono.h", ""
  "bits/cow_string.h", ""
  "bits/fs_dir.h", ""
  "bits/fs_fwd.h", ""
  "bits/fs_ops.h", ""
  "bits/fs_path.h", ""
  "bits/iterator_concepts.h", ""
  "bits/max_size_type.h", ""
  "bits/memory_resource.h", ""
  "bits/move_only_function.h", 
  "bits/ranges_algo.h", ""
  "bits/ranges_algobase.h", ""
  "bits/ranges_base.h", ""
  "bits/ranges_cmp.h", ""
  "bits/ranges_uninitialized.h", ""
  "bits/ranges_util.h", ""
  "bits/semaphore_base.h", ""
  "bits/std_thread.h", ""
  "bits/this_thread_sleep.h", ""
  "bits/uses_allocator_args.h", ""
  "bits/unique_lock.h", ""
  "bits/utility.h", ""

Furthermore, the following headers have no mapping:

  // Not clearly associated
  bits/c++0x_warning.h
  bits/enable_special_members.h
  bits/erase_if.h
  bits/node_handle.h
  
  // Not included anywhere
  bits/mofunc_impl.h
  bits/new_allocator.h
  bits/specfun.h

The search was limited to `bits/*.h` in GCC trunk.
I'll push a new revision after I have a chance to review the list a second time.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127184/new/

https://reviews.llvm.org/D127184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127184: [clangd] Add to header map

[Florian Albrechtskirchinger via Phabricator via cfe-commits]

falbrechtskirchinger added a comment.

Since `*.tcc` files are mapped as well and there are only a few of them 
missing, should I add them as well?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127184/new/

https://reviews.llvm.org/D127184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127184: [clangd] Add to header map

[Florian Albrechtskirchinger via Phabricator via cfe-commits]

falbrechtskirchinger updated this revision to Diff 436377.
falbrechtskirchinger added a comment.

Add more missing headers to the system header map


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127184/new/

https://reviews.llvm.org/D127184

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp

Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -181,19 +181,26 @@
   {"backward/auto_ptr.h", ""},
   {"backward/binders.h", ""},
   {"bits/algorithmfwd.h", ""},
+  {"bits/align.h", ""},
   {"bits/alloc_traits.h", ""},
   {"bits/allocated_ptr.h", ""},
   {"bits/allocator.h", ""},
   {"bits/atomic_base.h", ""},
   {"bits/atomic_lockfree_defines.h", ""},
   {"bits/atomic_futex.h", ""},
+  {"bits/atomic_timed_wait.h", ""},
+  {"bits/atomic_wait.h", ""},
   {"bits/basic_ios.h", ""},
   {"bits/basic_ios.tcc", ""},
   {"bits/basic_string.h", ""},
   {"bits/basic_string.tcc", ""},
+  {"bits/boost_concept_check.h", ""},
   {"bits/char_traits.h", ""},
+  {"bits/charconv.h", ""},
+  {"bits/chrono.h", ""},
   {"bits/codecvt.h", ""},
   {"bits/concept_check.h", ""},
+  {"bits/cow_string.h", ""},
   {"bits/cpp_type_traits.h", ""},
   {"bits/cxxabi_forced.h", ""},
   {"bits/deque.tcc", ""},
@@ -202,6 +209,10 @@
   {"bits/exception_ptr.h", ""},
   {"bits/forward_list.h", ""},
   {"bits/forward_list.tcc", ""},
+  {"bits/fs_dir.h", ""},
+  {"bits/fs_fwd.h", ""},
+  {"bits/fs_ops.h", ""},
+  {"bits/fs_path.h", ""},
   {"bits/fstream.tcc", ""},
   {"bits/functexcept.h", ""},
   {"bits/functional_hash.h", ""},
@@ -214,6 +225,7 @@
   {"bits/invoke.h", ""},
   {"bits/ios_base.h", ""},
   {"bits/istream.tcc", ""},
+  {"bits/iterator_concepts.h", ""},
   {"bits/list.tcc", ""},
   {"bits/locale_classes.h", ""},
   {"bits/locale_classes.tcc", ""},
@@ -224,8 +236,11 @@
   {"bits/locale_facets_nonio.tcc", ""},
   {"bits/localefwd.h", ""},
   {"bits/mask_array.h", ""},
+  {"bits/max_size_type.h", ""},
+  {"bits/memory_resource.h", ""},
   {"bits/memoryfwd.h", ""},
   {"bits/move.h", ""},
+  {"bits/move_only_function.h", ""},
   {"bits/nested_exception.h", ""},
   {"bits/ostream.tcc", ""},
   {"bits/ostream_insert.h", ""},
@@ -237,6 +252,12 @@
   {"bits/random.h", ""},
   {"bits/random.tcc", ""},
   {"bits/range_access.h", ""},
+  {"bits/ranges_algo.h", ""},
+  {"bits/ranges_algobase.h", ""},
+  {"bits/ranges_base.h", ""},
+  {"bits/ranges_cmp.h", ""},
+  {"bits/ranges_uninitialized.h", ""},
+  {"bits/ranges_util.h", ""},
   {"bits/refwrap.h", ""},
   {"bits/regex.h", ""},
   {"bits/regex_automaton.h", ""},
@@ -249,6 +270,7 @@
   {"bits/regex_grep_matcher.tcc", ""},
   {"bits/regex_nfa.h", ""},
   {"bits/regex_scanner.h", ""},
+  {"bits/semaphore_base.h", ""},
   {"bits/shared_ptr.h", ""},
   {"bits/shared_ptr_base.h", ""},
   {"bits/shared_ptr_atomic.h", ""},
@@ -257,6 +279,7 @@
   {"bits/std_abs.h", ""},
   {"bits/std_function.h", ""},
   {"bits/std_mutex.h", ""},
+  {"bits/std_thread.h", ""},
   {"bits/stl_algo.h", ""},
   {"bits/stl_algobase.h", ""},
   {"bits/stl_bvector.h", ""},
@@ -286,11 +309,15 @@
   {"bits/streambuf.tcc", ""},
   {"bits/streambuf_iterator.h", ""},
   {"bits/stringfwd.h", ""},
+  {"bits/this_thread_sleep.h", ""},
   {"bits/uniform_int_dist.h", ""},
+  {"bits/unique_lock.h", ""},
   {"bits/unique_ptr.h", ""},
   {"bits/unordered_map.h", ""},
   {"bits/unordered_set.h", ""},
   {"bits/uses_allocator.h", ""},
+  {"bits/uses_allocator_args.h", ""},
+  {"bits/utility.h", ""},
   {"bits/valarray_after.h", ""},
   {"bits/valarray_array.h", ""},
   {"bits/valarray_array.tcc", ""},
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127184: [clangd] Add to header map

[Florian Albrechtskirchinger via Phabricator via cfe-commits]

falbrechtskirchinger added a comment.

In D127184#3588226 , @nridge wrote:

> In D127184#3577165 , 
> @falbrechtskirchinger wrote:
>
>> bits/mofunc_impl.h
>
> I see this included from `bits/move_only_function.h`, so I think 
> `` 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 ``, we could add that. 
> (There are a few more implementation headers in `experimental/bits` which are 
> included by standard headers in `` that we could consider 
> adding.)

I did see that and excluded `> bits/specfun.h
>
> I see this included from ``.

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 , @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 (``)?
The current mappings are both very incomplete and also very wrong, or so I'd 
argue.
For example, `"ext/new_allocator.h"` maps to `` 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 ``.

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

That's a bit much for now.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127184/new/

https://reviews.llvm.org/D127184

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127184: [clangd] Add to header map

[Florian Albrechtskirchinger via Phabricator via cfe-commits]

falbrechtskirchinger created this revision.
falbrechtskirchinger added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
falbrechtskirchinger requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Header insertion inserts the internal  header when
completing, e.g., std::ranges::transform. By adding a mapping to the
 header to the system header map, the expected header is
inserted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127184

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp


Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -237,6 +237,7 @@
   {"bits/random.h", ""},
   {"bits/random.tcc", ""},
   {"bits/range_access.h", ""},
+  {"bits/ranges_algo.h", ""},
   {"bits/refwrap.h", ""},
   {"bits/regex.h", ""},
   {"bits/regex_automaton.h", ""},


Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -237,6 +237,7 @@
   {"bits/random.h", ""},
   {"bits/random.tcc", ""},
   {"bits/range_access.h", ""},
+  {"bits/ranges_algo.h", ""},
   {"bits/refwrap.h", ""},
   {"bits/regex.h", ""},
   {"bits/regex_automaton.h", ""},
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits