[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan updated this revision to Diff 496971.
zyounan added a comment.

Apply clang-format to StandardLibrary.cpp to fix pipeline failure


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

Files:
  clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
  clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc
  clang/unittests/Tooling/StandardLibraryTest.cpp

Index: clang/unittests/Tooling/StandardLibraryTest.cpp
===
--- clang/unittests/Tooling/StandardLibraryTest.cpp
+++ clang/unittests/Tooling/StandardLibraryTest.cpp
@@ -79,6 +79,23 @@
   EXPECT_FALSE(stdlib::Header::named("", stdlib::Lang::CXX));
 }
 
+TEST(StdlibTest, Experimental) {
+  EXPECT_FALSE(
+  stdlib::Header::named("", stdlib::Lang::C));
+  EXPECT_TRUE(
+  stdlib::Header::named("", stdlib::Lang::CXX));
+
+  auto Symbol = stdlib::Symbol::named("std::experimental::filesystem::",
+  "system_complete");
+  EXPECT_TRUE(Symbol);
+  EXPECT_EQ(Symbol->scope(), "std::experimental::filesystem::");
+  EXPECT_EQ(Symbol->name(), "system_complete");
+  EXPECT_EQ(Symbol->header(),
+stdlib::Header::named(""));
+  EXPECT_EQ(Symbol->qualifiedName(),
+"std::experimental::filesystem::system_complete");
+}
+
 TEST(StdlibTest, CCompat) {
   EXPECT_THAT(
   stdlib::Symbol::named("", "int16_t", stdlib::Lang::CXX)->headers(),
Index: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc
===
--- /dev/null
+++ clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc
@@ -0,0 +1,54 @@
+// These are derived from N4100[fs.filesystem.synopsis], final
+// draft for experimental filesystem.
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )
+SYMBOL(canonical, std::experimental::filesystem::, )
+SYMBOL(copy, std::experimental::filesystem::, )
+SYMBOL(copy_file, std::experimental::filesystem::, )
+SYMBOL(copy_options, std::experimental::filesystem::, )
+SYMBOL(copy_symlink, std::experimental::filesystem::, )
+SYMBOL(create_directories, std::experimental::filesystem::, )
+SYMBOL(create_directory, std::experimental::filesystem::, )
+SYMBOL(create_directory_symlink, std::experimental::filesystem::, )
+SYMBOL(create_hard_link, std::experimental::filesystem::, )
+SYMBOL(create_symlink, std::experimental::filesystem::, )
+SYMBOL(current_path, std::experimental::filesystem::, )
+SYMBOL(directory_entry, std::experimental::filesystem::, )
+SYMBOL(directory_iterator, std::experimental::filesystem::, )
+SYMBOL(directory_options, std::experimental::filesystem::, )
+SYMBOL(equivalent, std::experimental::filesystem::, )
+SYMBOL(exists, std::experimental::filesystem::, )
+SYMBOL(file_size, std::experimental::filesystem::, )
+SYMBOL(file_status, std::experimental::filesystem::, )
+SYMBOL(file_time_type, std::experimental::filesystem::, )
+SYMBOL(file_type, std::experimental::filesystem::, )
+SYMBOL(filesystem_error, std::experimental::filesystem::, )
+SYMBOL(hard_link_count, std::experimental::filesystem::, )
+SYMBOL(is_block_file, std::experimental::filesystem::, )
+SYMBOL(is_character_file, std::experimental::filesystem::, )
+SYMBOL(is_directory, std::experimental::filesystem::, )
+SYMBOL(is_empty, std::experimental::filesystem::, )
+SYMBOL(is_fifo, std::experimental::filesystem::, )
+SYMBOL(is_other, std::experimental::filesystem::, )
+SYMBOL(is_regular_file, std::experimental::filesystem::, )
+SYMBOL(is_socket, std::experimental::filesystem::, )
+SYMBOL(is_symlink, std::experimental::filesystem::, )
+SYMBOL(last_write_time, std::experimental::filesystem::, )
+SYMBOL(path, std::experimental::filesystem::, )
+SYMBOL(permissions, std::experimental::filesystem::, )
+SYMBOL(perms, std::experimental::filesystem::, )
+SYMBOL(read_symlink, std::experimental::filesystem::, )
+SYMBOL(recursive_directory_iterator, std::experimental::filesystem::, )
+SYMBOL(remove, std::experimental::filesystem::, )
+SYMBOL(remove_all, std::experimental::filesystem::, )
+SYMBOL(rename, std::experimental::filesystem::, )
+SYMBOL(resize_file, std::experimental::filesystem::, )
+SYMBOL(space, std::experimental::filesystem::, )
+SYMBOL(space_info, std::experimental::filesystem::, )
+SYMBOL(status, std::experimental::filesystem::, )
+SYMBOL(status_known, std::experimental::filesystem::, )
+SYMBOL(symlink_status, std::experimental::filesystem::, )
+SYMBOL(system_complete, std::experimental::filesystem::, )
+SYMBOL(temp_directory_path, std::experimental::filesystem::, )
+SYMBOL(u8path, std::experimental::filesystem::, )
+// clang-format on
Index: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
===
--- clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
+++ clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
@@ -61,8 +61,9 @@
 #include 

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan added inline comments.



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc:6
+// whatever.
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )

zyounan wrote:
> kadircet wrote:
> > zyounan wrote:
> > > kadircet wrote:
> > > > zyounan wrote:
> > > > > kadircet wrote:
> > > > > > can you strip clang-format pragmas to be similar to other mapping 
> > > > > > files.
> > > > > I'd love to if possible, but for some reason clang-format would add 
> > > > > extra spaces before and after the slash, ` > > > > filesystem>`, which looks ugly and I don't expect it right.
> > > > i don't follow, why do you **need** to run clang-format on this file? 
> > > > symbols are already ordered
> > > Specifically, it is `git clang-format`. The pipeline would fail if this 
> > > command does modify some files IIRC.
> > not sure what workflow you're using but clang-format is not a mandatory 
> > part of the workflow.
> > recommended way is updating/uploading patches using arcanist, whose config 
> > only contains files with one of the `cc|h|cpp` extensions, see 
> > https://github.com/llvm/llvm-project/blob/main/.arclint.
> > 
> > anyways, happy to land this for you if it's not easy to disable 
> > clang-format linting on your workflow for whatever reason.
> I've looked into the script of the running pipeline, it shows that it is 
> running `${SRC}/scripts/premerge_checks.py --check-clang-format`, which would 
> [run `git 
> clang-format`](https://github.com/google/llvm-premerge-checks/blob/abe5c8991b5e81f0182528ff8ce515ba89a66c0a/scripts/premerge_checks.py#L163).
>  I'm using arcanist with default config, and I don't know if I'm missing 
> something to skip this. (Perhaps it is because the rule you mentioned doesn't 
> exclude `.inc` files, and we didn't write any slashes in such files either.)
...Or it is the rule defined in pipeline doesn't exclude them... 
https://github.com/google/llvm-premerge-checks/blob/abe5c8991b5e81f0182528ff8ce515ba89a66c0a/scripts/clang-format.ignore



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan added a comment.

Thank you for your detailed explanation and sorry again for my dumb mistake 
before. :)




Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc:6
+// whatever.
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )

kadircet wrote:
> zyounan wrote:
> > kadircet wrote:
> > > zyounan wrote:
> > > > kadircet wrote:
> > > > > can you strip clang-format pragmas to be similar to other mapping 
> > > > > files.
> > > > I'd love to if possible, but for some reason clang-format would add 
> > > > extra spaces before and after the slash, ``, 
> > > > which looks ugly and I don't expect it right.
> > > i don't follow, why do you **need** to run clang-format on this file? 
> > > symbols are already ordered
> > Specifically, it is `git clang-format`. The pipeline would fail if this 
> > command does modify some files IIRC.
> not sure what workflow you're using but clang-format is not a mandatory part 
> of the workflow.
> recommended way is updating/uploading patches using arcanist, whose config 
> only contains files with one of the `cc|h|cpp` extensions, see 
> https://github.com/llvm/llvm-project/blob/main/.arclint.
> 
> anyways, happy to land this for you if it's not easy to disable clang-format 
> linting on your workflow for whatever reason.
I've looked into the script of the running pipeline, it shows that it is 
running `${SRC}/scripts/premerge_checks.py --check-clang-format`, which would 
[run `git 
clang-format`](https://github.com/google/llvm-premerge-checks/blob/abe5c8991b5e81f0182528ff8ce515ba89a66c0a/scripts/premerge_checks.py#L163).
 I'm using arcanist with default config, and I don't know if I'm missing 
something to skip this. (Perhaps it is because the rule you mentioned doesn't 
exclude `.inc` files, and we didn't write any slashes in such files either.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks!




Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc:6
+// whatever.
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )

zyounan wrote:
> kadircet wrote:
> > zyounan wrote:
> > > kadircet wrote:
> > > > can you strip clang-format pragmas to be similar to other mapping files.
> > > I'd love to if possible, but for some reason clang-format would add extra 
> > > spaces before and after the slash, ``, which 
> > > looks ugly and I don't expect it right.
> > i don't follow, why do you **need** to run clang-format on this file? 
> > symbols are already ordered
> Specifically, it is `git clang-format`. The pipeline would fail if this 
> command does modify some files IIRC.
not sure what workflow you're using but clang-format is not a mandatory part of 
the workflow.
recommended way is updating/uploading patches using arcanist, whose config only 
contains files with one of the `cc|h|cpp` extensions, see 
https://github.com/llvm/llvm-project/blob/main/.arclint.

anyways, happy to land this for you if it's not easy to disable clang-format 
linting on your workflow for whatever reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan added inline comments.



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc:6
+// whatever.
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )

kadircet wrote:
> zyounan wrote:
> > kadircet wrote:
> > > can you strip clang-format pragmas to be similar to other mapping files.
> > I'd love to if possible, but for some reason clang-format would add extra 
> > spaces before and after the slash, ``, which 
> > looks ugly and I don't expect it right.
> i don't follow, why do you **need** to run clang-format on this file? symbols 
> are already ordered
Specifically, it is `git clang-format`. The pipeline would fail if this command 
does modify some files IIRC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan added inline comments.



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc:1
+// These symbols are exported from N4100[fs.filesystem.synopsis], the final
+// draft for experimental filesystem. Note that not all of these symbols were

kadircet wrote:
> i think comment is a little too verbose. can you just say:
> ```
> These are derived from N4100[fs.filesystem.synopsis], final draft for 
> experimental filesystem.
> ```
> 
> There's no need for mentioning that these became the standard in C++17 or 
> being a cornerstone for stdlib implementations. As they won't necessarily be 
> true for other technical specs nor if we were adding this pre-c++17 
> standardisation. But we'd still like to have these widely adapted symbols 
> included in the mapping to make sure we're not generating false negatives.
Well, I'm not quite sure if I should address these small differences between TS 
and C++17. But FWIW, the verbosity should go away.



Comment at: clang/unittests/Tooling/StandardLibraryTest.cpp:93
+
+  // system_complete is replaced by std::filesystem::absolute
+  Symbol = stdlib::Symbol::named("std::filesystem::", "system_complete");

kadircet wrote:
> i don't think there's much point in asserting these "meta" details about the 
> mapping (same for the test below).
They're used to emphasize the slight difference. They can be removed if you 
don't think it necessary :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan updated this revision to Diff 496963.
zyounan marked 4 inline comments as done.
zyounan added a comment.

Revise


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

Files:
  clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
  clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc
  clang/unittests/Tooling/StandardLibraryTest.cpp

Index: clang/unittests/Tooling/StandardLibraryTest.cpp
===
--- clang/unittests/Tooling/StandardLibraryTest.cpp
+++ clang/unittests/Tooling/StandardLibraryTest.cpp
@@ -79,6 +79,23 @@
   EXPECT_FALSE(stdlib::Header::named("", stdlib::Lang::CXX));
 }
 
+TEST(StdlibTest, Experimental) {
+  EXPECT_FALSE(
+  stdlib::Header::named("", stdlib::Lang::C));
+  EXPECT_TRUE(
+  stdlib::Header::named("", stdlib::Lang::CXX));
+
+  auto Symbol = stdlib::Symbol::named("std::experimental::filesystem::",
+  "system_complete");
+  EXPECT_TRUE(Symbol);
+  EXPECT_EQ(Symbol->scope(), "std::experimental::filesystem::");
+  EXPECT_EQ(Symbol->name(), "system_complete");
+  EXPECT_EQ(Symbol->header(),
+stdlib::Header::named(""));
+  EXPECT_EQ(Symbol->qualifiedName(),
+"std::experimental::filesystem::system_complete");
+}
+
 TEST(StdlibTest, CCompat) {
   EXPECT_THAT(
   stdlib::Symbol::named("", "int16_t", stdlib::Lang::CXX)->headers(),
Index: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc
===
--- /dev/null
+++ clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc
@@ -0,0 +1,54 @@
+// These are derived from N4100[fs.filesystem.synopsis], final
+// draft for experimental filesystem.
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )
+SYMBOL(canonical, std::experimental::filesystem::, )
+SYMBOL(copy, std::experimental::filesystem::, )
+SYMBOL(copy_file, std::experimental::filesystem::, )
+SYMBOL(copy_options, std::experimental::filesystem::, )
+SYMBOL(copy_symlink, std::experimental::filesystem::, )
+SYMBOL(create_directories, std::experimental::filesystem::, )
+SYMBOL(create_directory, std::experimental::filesystem::, )
+SYMBOL(create_directory_symlink, std::experimental::filesystem::, )
+SYMBOL(create_hard_link, std::experimental::filesystem::, )
+SYMBOL(create_symlink, std::experimental::filesystem::, )
+SYMBOL(current_path, std::experimental::filesystem::, )
+SYMBOL(directory_entry, std::experimental::filesystem::, )
+SYMBOL(directory_iterator, std::experimental::filesystem::, )
+SYMBOL(directory_options, std::experimental::filesystem::, )
+SYMBOL(equivalent, std::experimental::filesystem::, )
+SYMBOL(exists, std::experimental::filesystem::, )
+SYMBOL(file_size, std::experimental::filesystem::, )
+SYMBOL(file_status, std::experimental::filesystem::, )
+SYMBOL(file_time_type, std::experimental::filesystem::, )
+SYMBOL(file_type, std::experimental::filesystem::, )
+SYMBOL(filesystem_error, std::experimental::filesystem::, )
+SYMBOL(hard_link_count, std::experimental::filesystem::, )
+SYMBOL(is_block_file, std::experimental::filesystem::, )
+SYMBOL(is_character_file, std::experimental::filesystem::, )
+SYMBOL(is_directory, std::experimental::filesystem::, )
+SYMBOL(is_empty, std::experimental::filesystem::, )
+SYMBOL(is_fifo, std::experimental::filesystem::, )
+SYMBOL(is_other, std::experimental::filesystem::, )
+SYMBOL(is_regular_file, std::experimental::filesystem::, )
+SYMBOL(is_socket, std::experimental::filesystem::, )
+SYMBOL(is_symlink, std::experimental::filesystem::, )
+SYMBOL(last_write_time, std::experimental::filesystem::, )
+SYMBOL(path, std::experimental::filesystem::, )
+SYMBOL(permissions, std::experimental::filesystem::, )
+SYMBOL(perms, std::experimental::filesystem::, )
+SYMBOL(read_symlink, std::experimental::filesystem::, )
+SYMBOL(recursive_directory_iterator, std::experimental::filesystem::, )
+SYMBOL(remove, std::experimental::filesystem::, )
+SYMBOL(remove_all, std::experimental::filesystem::, )
+SYMBOL(rename, std::experimental::filesystem::, )
+SYMBOL(resize_file, std::experimental::filesystem::, )
+SYMBOL(space, std::experimental::filesystem::, )
+SYMBOL(space_info, std::experimental::filesystem::, )
+SYMBOL(status, std::experimental::filesystem::, )
+SYMBOL(status_known, std::experimental::filesystem::, )
+SYMBOL(symlink_status, std::experimental::filesystem::, )
+SYMBOL(system_complete, std::experimental::filesystem::, )
+SYMBOL(temp_directory_path, std::experimental::filesystem::, )
+SYMBOL(u8path, std::experimental::filesystem::, )
+// clang-format on
Index: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
===
--- clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
+++ clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
@@ -63,6 +63,7 @@
   case Lang::CXX:
 #include 

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc:6
+// whatever.
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )

zyounan wrote:
> kadircet wrote:
> > can you strip clang-format pragmas to be similar to other mapping files.
> I'd love to if possible, but for some reason clang-format would add extra 
> spaces before and after the slash, ``, which looks 
> ugly and I don't expect it right.
i don't follow, why do you **need** to run clang-format on this file? symbols 
are already ordered


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan added inline comments.



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc:6
+// whatever.
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )

kadircet wrote:
> can you strip clang-format pragmas to be similar to other mapping files.
I'd love to if possible, but for some reason clang-format would add extra 
spaces before and after the slash, ``, which looks 
ugly and I don't expect it right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks a lot! mostly LG couple more nits




Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc:1
+// These symbols are exported from N4100[fs.filesystem.synopsis], the final
+// draft for experimental filesystem. Note that not all of these symbols were

i think comment is a little too verbose. can you just say:
```
These are derived from N4100[fs.filesystem.synopsis], final draft for 
experimental filesystem.
```

There's no need for mentioning that these became the standard in C++17 or being 
a cornerstone for stdlib implementations. As they won't necessarily be true for 
other technical specs nor if we were adding this pre-c++17 standardisation. But 
we'd still like to have these widely adapted symbols included in the mapping to 
make sure we're not generating false negatives.



Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc:6
+// whatever.
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )

can you strip clang-format pragmas to be similar to other mapping files.



Comment at: clang/unittests/Tooling/StandardLibraryTest.cpp:83
+TEST(StdlibTest, Experimental) {
+  EXPECT_TRUE(
+  stdlib::Header::named("", stdlib::Lang::CXX));

another EXPECT_FALSE with `Lang::C` would also be important to make sure we're 
not adding these for C mappings by mistake.



Comment at: clang/unittests/Tooling/StandardLibraryTest.cpp:91
+  EXPECT_EQ(Symbol->name(), "path");
+  EXPECT_EQ(Symbol->qualifiedName(), "std::experimental::filesystem::path");
+

can you also check for `Symbol->headers()`



Comment at: clang/unittests/Tooling/StandardLibraryTest.cpp:93
+
+  // system_complete is replaced by std::filesystem::absolute
+  Symbol = stdlib::Symbol::named("std::filesystem::", "system_complete");

i don't think there's much point in asserting these "meta" details about the 
mapping (same for the test below).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan updated this revision to Diff 496934.
zyounan added a comment.

Rebase to main && Move tests to clang/Tooling


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

Files:
  clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
  clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc
  clang/unittests/Tooling/StandardLibraryTest.cpp

Index: clang/unittests/Tooling/StandardLibraryTest.cpp
===
--- clang/unittests/Tooling/StandardLibraryTest.cpp
+++ clang/unittests/Tooling/StandardLibraryTest.cpp
@@ -79,6 +79,41 @@
   EXPECT_FALSE(stdlib::Header::named("", stdlib::Lang::CXX));
 }
 
+TEST(StdlibTest, Experimental) {
+  EXPECT_TRUE(
+  stdlib::Header::named("", stdlib::Lang::CXX));
+
+  auto Symbol =
+  stdlib::Symbol::named("std::experimental::filesystem::", "path");
+  EXPECT_TRUE(Symbol);
+  EXPECT_EQ(Symbol->scope(), "std::experimental::filesystem::");
+  EXPECT_EQ(Symbol->name(), "path");
+  EXPECT_EQ(Symbol->qualifiedName(), "std::experimental::filesystem::path");
+
+  // system_complete is replaced by std::filesystem::absolute
+  Symbol = stdlib::Symbol::named("std::filesystem::", "system_complete");
+  EXPECT_FALSE(Symbol);
+  Symbol = stdlib::Symbol::named("std::experimental::filesystem::",
+ "system_complete");
+  EXPECT_TRUE(Symbol);
+  EXPECT_EQ(Symbol->scope(), "std::experimental::filesystem::");
+  EXPECT_EQ(Symbol->name(), "system_complete");
+  EXPECT_EQ(Symbol->qualifiedName(),
+"std::experimental::filesystem::system_complete");
+
+  // relative-path-related operation is introduced by P0219
+  Symbol = stdlib::Symbol::named("std::filesystem::", "relative");
+  EXPECT_TRUE(Symbol);
+  Symbol = stdlib::Symbol::named("std::experimental::filesystem::", "relative");
+  EXPECT_FALSE(Symbol);
+  Symbol =
+  stdlib::Symbol::named("std::experimental::filesystem::", "proximate");
+  EXPECT_FALSE(Symbol);
+  Symbol = stdlib::Symbol::named("std::experimental::filesystem::",
+ "weakly_canonical");
+  EXPECT_FALSE(Symbol);
+}
+
 TEST(StdlibTest, CCompat) {
   EXPECT_THAT(
   stdlib::Symbol::named("", "int16_t", stdlib::Lang::CXX)->headers(),
Index: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc
===
--- /dev/null
+++ clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc
@@ -0,0 +1,57 @@
+// These symbols are exported from N4100[fs.filesystem.synopsis], the final
+// draft for experimental filesystem. Note that not all of these symbols were
+// merged into C++17 (See P0492 and related proposals for details). But the
+// cornerstone for mainstream Stdlib TS filesystem implementation is N4100,
+// whatever.
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )
+SYMBOL(canonical, std::experimental::filesystem::, )
+SYMBOL(copy, std::experimental::filesystem::, )
+SYMBOL(copy_file, std::experimental::filesystem::, )
+SYMBOL(copy_options, std::experimental::filesystem::, )
+SYMBOL(copy_symlink, std::experimental::filesystem::, )
+SYMBOL(create_directories, std::experimental::filesystem::, )
+SYMBOL(create_directory, std::experimental::filesystem::, )
+SYMBOL(create_directory_symlink, std::experimental::filesystem::, )
+SYMBOL(create_hard_link, std::experimental::filesystem::, )
+SYMBOL(create_symlink, std::experimental::filesystem::, )
+SYMBOL(current_path, std::experimental::filesystem::, )
+SYMBOL(directory_entry, std::experimental::filesystem::, )
+SYMBOL(directory_iterator, std::experimental::filesystem::, )
+SYMBOL(directory_options, std::experimental::filesystem::, )
+SYMBOL(equivalent, std::experimental::filesystem::, )
+SYMBOL(exists, std::experimental::filesystem::, )
+SYMBOL(file_size, std::experimental::filesystem::, )
+SYMBOL(file_status, std::experimental::filesystem::, )
+SYMBOL(file_time_type, std::experimental::filesystem::, )
+SYMBOL(file_type, std::experimental::filesystem::, )
+SYMBOL(filesystem_error, std::experimental::filesystem::, )
+SYMBOL(hard_link_count, std::experimental::filesystem::, )
+SYMBOL(is_block_file, std::experimental::filesystem::, )
+SYMBOL(is_character_file, std::experimental::filesystem::, )
+SYMBOL(is_directory, std::experimental::filesystem::, )
+SYMBOL(is_empty, std::experimental::filesystem::, )
+SYMBOL(is_fifo, std::experimental::filesystem::, )
+SYMBOL(is_other, std::experimental::filesystem::, )
+SYMBOL(is_regular_file, std::experimental::filesystem::, )
+SYMBOL(is_socket, std::experimental::filesystem::, )
+SYMBOL(is_symlink, std::experimental::filesystem::, )
+SYMBOL(last_write_time, std::experimental::filesystem::, )
+SYMBOL(path, std::experimental::filesystem::, )
+SYMBOL(permissions, std::experimental::filesystem::, )
+SYMBOL(perms, std::experimental::filesystem::, )
+SYMBOL(read_symlink, std::experimental::filesystem::, )

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan added a comment.

Sorry for the late update, I'm rebasing my branch now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Thanks for updating this, as I mentioned in 
https://reviews.llvm.org/D143319#4115186, we should put these symbols into 
their own symbol map. ATM `StdSymbolMap.inc` is still generated by an automated 
tool and shouldn't be modified by hand.
So can you rather put these symbols into 
`llvm/llvm-project/clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc` ? 
you also need to rebase your branch, we've moved these mappings from `include` 
directory to `lib`. they're implementation details now, and not public 
interfaces.
After putting it into a new file, you'll also need to include it in 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp#L65.

As for tests, rather than using clangd, can you introduce tests into 
StandardLibrary instead in 
https://github.com/llvm/llvm-project/blob/main/clang/unittests/Tooling/StandardLibraryTest.cpp
 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan updated this revision to Diff 496875.
zyounan added a comment.

Revise symbols from N4100


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

Files:
  clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
  clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc


Index: clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
===
--- clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
+++ clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc
@@ -1465,6 +1465,63 @@
 SYMBOL(temp_directory_path, std::filesystem::, )
 SYMBOL(u8path, std::filesystem::, )
 SYMBOL(weakly_canonical, std::filesystem::, )
+// These symbols are exported from N4100[fs.filesystem.synopsis], the final
+// draft for experimental filesystem. Note that not all of these symbols were
+// merged into C++17 (See P0492 and related proposals for details). But the
+// cornerstone for mainstream Stdlib TS filesystem implementation is N4100,
+// whatever.
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )
+SYMBOL(canonical, std::experimental::filesystem::, )
+SYMBOL(copy, std::experimental::filesystem::, )
+SYMBOL(copy_file, std::experimental::filesystem::, )
+SYMBOL(copy_options, std::experimental::filesystem::, 
)
+SYMBOL(copy_symlink, std::experimental::filesystem::, 
)
+SYMBOL(create_directories, std::experimental::filesystem::, 
)
+SYMBOL(create_directory, std::experimental::filesystem::, 
)
+SYMBOL(create_directory_symlink, std::experimental::filesystem::, 
)
+SYMBOL(create_hard_link, std::experimental::filesystem::, 
)
+SYMBOL(create_symlink, std::experimental::filesystem::, 
)
+SYMBOL(current_path, std::experimental::filesystem::, 
)
+SYMBOL(directory_entry, std::experimental::filesystem::, 
)
+SYMBOL(directory_iterator, std::experimental::filesystem::, 
)
+SYMBOL(directory_options, std::experimental::filesystem::, 
)
+SYMBOL(equivalent, std::experimental::filesystem::, )
+SYMBOL(exists, std::experimental::filesystem::, )
+SYMBOL(file_size, std::experimental::filesystem::, )
+SYMBOL(file_status, std::experimental::filesystem::, )
+SYMBOL(file_time_type, std::experimental::filesystem::, 
)
+SYMBOL(file_type, std::experimental::filesystem::, )
+SYMBOL(filesystem_error, std::experimental::filesystem::, 
)
+SYMBOL(hard_link_count, std::experimental::filesystem::, 
)
+SYMBOL(is_block_file, std::experimental::filesystem::, 
)
+SYMBOL(is_character_file, std::experimental::filesystem::, 
)
+SYMBOL(is_directory, std::experimental::filesystem::, 
)
+SYMBOL(is_empty, std::experimental::filesystem::, )
+SYMBOL(is_fifo, std::experimental::filesystem::, )
+SYMBOL(is_other, std::experimental::filesystem::, )
+SYMBOL(is_regular_file, std::experimental::filesystem::, 
)
+SYMBOL(is_socket, std::experimental::filesystem::, )
+SYMBOL(is_symlink, std::experimental::filesystem::, )
+SYMBOL(last_write_time, std::experimental::filesystem::, 
)
+SYMBOL(path, std::experimental::filesystem::, )
+SYMBOL(permissions, std::experimental::filesystem::, )
+SYMBOL(perms, std::experimental::filesystem::, )
+SYMBOL(read_symlink, std::experimental::filesystem::, 
)
+SYMBOL(recursive_directory_iterator, std::experimental::filesystem::, 
)
+SYMBOL(remove, std::experimental::filesystem::, )
+SYMBOL(remove_all, std::experimental::filesystem::, )
+SYMBOL(rename, std::experimental::filesystem::, )
+SYMBOL(resize_file, std::experimental::filesystem::, )
+SYMBOL(space, std::experimental::filesystem::, )
+SYMBOL(space_info, std::experimental::filesystem::, )
+SYMBOL(status, std::experimental::filesystem::, )
+SYMBOL(status_known, std::experimental::filesystem::, 
)
+SYMBOL(symlink_status, std::experimental::filesystem::, 
)
+SYMBOL(system_complete, std::experimental::filesystem::, 
)
+SYMBOL(temp_directory_path, std::experimental::filesystem::, 
)
+SYMBOL(u8path, std::experimental::filesystem::, )
+// clang-format on
 SYMBOL(basic_string, std::pmr::, )
 SYMBOL(deque, std::pmr::, )
 SYMBOL(forward_list, std::pmr::, )
Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -62,6 +62,27 @@
   EXPECT_EQ("", CI.mapHeader(File));
 }
 
+TEST(CanonicalIncludesTest, CXXStandardLibraryExperimentalFilesystem) {
+  CanonicalIncludes CI;
+  auto Language = LangOptions();
+  Language.CPlusPlus = true;
+  CI.addSystemHeadersMapping(Language);
+
+  EXPECT_EQ("",
+CI.mapSymbol("std::experimental::filesystem::path"));
+  EXPECT_EQ("",
+CI.mapSymbol("std::experimental::filesystem::directory_iterator"));
+  // system_complete is replaced by std::filesystem::absolute
+  EXPECT_EQ("",
+CI.mapSymbol("std::experimental::filesystem::system_complete"));
+  

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan added a comment.

In D142836#4089721 , @kadircet wrote:

> And to follow up, I definitely see the problem you're facing and it's 
> something we'd like to address as well, just not in the way you proposed.
>
> This falls under the bucket of "we might have symbols missing from our stdlib 
> mappings and should try to compensate for that", e.g. we should probably try 
> disabling include-insertion for such cases where we have a high confidence 
> that the symbol is part of stdlib but we don't have an exact mapping in our 
> symbol database to remedy the issue around inserting "wrong" includes.

Yes, this is another issue I want to address, but you've already pointed out 
that the parser is improving.

The cppref archive page is not always up to date. As of now, the official 
version is still dumped in 2019, that means we're likely missing (partial) 
symbols from C++20/23, e.g. `std::expected`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan added a comment.

Another question is that file `clang/Tooling/Inclusions/StdSymbolMap.inc` seems 
to be out-of-date. What's worse, the archive of cppref doesn't always update on 
time 
The latest version from official is published in 2019, while the unofficial 
version is 2022-06.
That means, symbols from C++23, like `std::expected` will be missing from this 
file.

From the aspect of maintenance, shall we consider a more optimal approach to 
keep update?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

And to follow up, I definitely see the problem you're facing and it's something 
we'd like to address as well, just not in the way you proposed.

This falls under the bucket of "we might have symbols missing from our stdlib 
mappings and should try to compensate for that", e.g. we should probably try 
disabling include-insertion for such cases where we have a high confidence that 
the symbol is part of stdlib but we don't have an exact mapping in our symbol 
database to remedy the issue around inserting "wrong" includes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a subscriber: VitaNuo.
kadircet added a comment.

thanks a lot for the patch, we're already having some efforts right now to 
improve our cppreference parsing and get to a more complete set of symbols 
(@VitaNuo for visibility, who drives these efforts).

Regarding the experimental symbols, first of all I am not sure if it's a good 
idea to include them in clangd (or other clang-tools) at all. As a clang-tool 
binary can stay around for years (e.g. people can use 3 years old clangd) and 
symbols under `std::experimental` are subject to change throughout that period, 
hence the information provided is going to be vastly different than what the 
truth is in that time window (also i don't know if they'll be "same" across 
libc++ vs libstdc++ implementations).
In addition to that, AFAICT there's no great index page to parse for 
technical-specs hence we might indeed be forced to manually curate these 
symbols and that's something we've been trying to avoid due to maintenance 
costs. In the case of technical-specs this cost will increase, because they're 
subject to change a lot more frequent than regular std symbols and value is 
even less as these are not used by majority of the developers.

So I am afraid this is not some extra complexity we can take in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

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

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan updated this revision to Diff 493195.
zyounan added a comment.

Format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
  clang/include/clang/Tooling/Inclusions/TsStdSymbolMap.inc

Index: clang/include/clang/Tooling/Inclusions/TsStdSymbolMap.inc
===
--- /dev/null
+++ clang/include/clang/Tooling/Inclusions/TsStdSymbolMap.inc
@@ -0,0 +1,58 @@
+// FIXME: Add a parser for experimental pages to
+// `clang/tools/include-mapping/cppreference_parser.py`
+
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )
+SYMBOL(canonical, std::experimental::filesystem::, )
+SYMBOL(copy, std::experimental::filesystem::, )
+SYMBOL(copy_file, std::experimental::filesystem::, )
+SYMBOL(copy_options, std::experimental::filesystem::, )
+SYMBOL(copy_symlink, std::experimental::filesystem::, )
+SYMBOL(create_directories, std::experimental::filesystem::, )
+SYMBOL(create_directory, std::experimental::filesystem::, )
+SYMBOL(create_directory_symlink, std::experimental::filesystem::, )
+SYMBOL(create_hard_link, std::experimental::filesystem::, )
+SYMBOL(create_symlink, std::experimental::filesystem::, )
+SYMBOL(current_path, std::experimental::filesystem::, )
+SYMBOL(directory_entry, std::experimental::filesystem::, )
+SYMBOL(directory_iterator, std::experimental::filesystem::, )
+SYMBOL(directory_options, std::experimental::filesystem::, )
+SYMBOL(equivalent, std::experimental::filesystem::, )
+SYMBOL(exists, std::experimental::filesystem::, )
+SYMBOL(file_size, std::experimental::filesystem::, )
+SYMBOL(file_status, std::experimental::filesystem::, )
+SYMBOL(file_time_type, std::experimental::filesystem::, )
+SYMBOL(file_type, std::experimental::filesystem::, )
+SYMBOL(filesystem_error, std::experimental::filesystem::, )
+SYMBOL(hard_link_count, std::experimental::filesystem::, )
+SYMBOL(is_block_file, std::experimental::filesystem::, )
+SYMBOL(is_character_file, std::experimental::filesystem::, )
+SYMBOL(is_directory, std::experimental::filesystem::, )
+SYMBOL(is_empty, std::experimental::filesystem::, )
+SYMBOL(is_fifo, std::experimental::filesystem::, )
+SYMBOL(is_other, std::experimental::filesystem::, )
+SYMBOL(is_regular_file, std::experimental::filesystem::, )
+SYMBOL(is_socket, std::experimental::filesystem::, )
+SYMBOL(is_symlink, std::experimental::filesystem::, )
+SYMBOL(last_write_time, std::experimental::filesystem::, )
+SYMBOL(path, std::experimental::filesystem::, )
+SYMBOL(perm_options, std::experimental::filesystem::, )
+SYMBOL(permissions, std::experimental::filesystem::, )
+SYMBOL(perms, std::experimental::filesystem::, )
+SYMBOL(proximate, std::experimental::filesystem::, )
+SYMBOL(read_symlink, std::experimental::filesystem::, )
+SYMBOL(recursive_directory_iterator, std::experimental::filesystem::, )
+SYMBOL(relative, std::experimental::filesystem::, )
+SYMBOL(remove, std::experimental::filesystem::, )
+SYMBOL(remove_all, std::experimental::filesystem::, )
+SYMBOL(rename, std::experimental::filesystem::, )
+SYMBOL(resize_file, std::experimental::filesystem::, )
+SYMBOL(space, std::experimental::filesystem::, )
+SYMBOL(space_info, std::experimental::filesystem::, )
+SYMBOL(status, std::experimental::filesystem::, )
+SYMBOL(status_known, std::experimental::filesystem::, )
+SYMBOL(symlink_status, std::experimental::filesystem::, )
+SYMBOL(temp_directory_path, std::experimental::filesystem::, )
+SYMBOL(u8path, std::experimental::filesystem::, )
+SYMBOL(weakly_canonical, std::experimental::filesystem::, )
+// clang-format on
Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -62,6 +62,23 @@
   EXPECT_EQ("", CI.mapHeader(File));
 }
 
+TEST(CanonicalIncludesTest, CXXSymbolsFromTS) {
+  CanonicalIncludes CI;
+  auto Language = LangOptions();
+  Language.CPlusPlus = true;
+  CI.addSystemHeadersMapping(Language);
+
+  EXPECT_EQ("",
+CI.mapSymbol("std::experimental::filesystem::path"));
+  EXPECT_EQ("",
+CI.mapSymbol("std::experimental::filesystem::file_type"));
+  EXPECT_EQ("",
+CI.mapSymbol("std::experimental::filesystem::file_status"));
+  EXPECT_EQ("",
+CI.mapSymbol(
+"std::experimental::filesystem::recursive_directory_iterator"));
+}
+
 TEST(CanonicalIncludesTest, PathMapping) {
   auto InMemFS = llvm::makeIntrusiveRefCnt();
   FileManager Files(FileSystemOptions(), InMemFS);
Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- 

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan updated this revision to Diff 493081.
zyounan added a comment.

Do not format inc file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142836

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
  clang/include/clang/Tooling/Inclusions/TsStdSymbolMap.inc

Index: clang/include/clang/Tooling/Inclusions/TsStdSymbolMap.inc
===
--- /dev/null
+++ clang/include/clang/Tooling/Inclusions/TsStdSymbolMap.inc
@@ -0,0 +1,57 @@
+// FIXME: Add a parser for experimental pages to `clang/tools/include-mapping/cppreference_parser.py`
+
+// clang-format off
+SYMBOL(absolute, std::experimental::filesystem::, )
+SYMBOL(canonical, std::experimental::filesystem::, )
+SYMBOL(copy, std::experimental::filesystem::, )
+SYMBOL(copy_file, std::experimental::filesystem::, )
+SYMBOL(copy_options, std::experimental::filesystem::, )
+SYMBOL(copy_symlink, std::experimental::filesystem::, )
+SYMBOL(create_directories, std::experimental::filesystem::, )
+SYMBOL(create_directory, std::experimental::filesystem::, )
+SYMBOL(create_directory_symlink, std::experimental::filesystem::, )
+SYMBOL(create_hard_link, std::experimental::filesystem::, )
+SYMBOL(create_symlink, std::experimental::filesystem::, )
+SYMBOL(current_path, std::experimental::filesystem::, )
+SYMBOL(directory_entry, std::experimental::filesystem::, )
+SYMBOL(directory_iterator, std::experimental::filesystem::, )
+SYMBOL(directory_options, std::experimental::filesystem::, )
+SYMBOL(equivalent, std::experimental::filesystem::, )
+SYMBOL(exists, std::experimental::filesystem::, )
+SYMBOL(file_size, std::experimental::filesystem::, )
+SYMBOL(file_status, std::experimental::filesystem::, )
+SYMBOL(file_time_type, std::experimental::filesystem::, )
+SYMBOL(file_type, std::experimental::filesystem::, )
+SYMBOL(filesystem_error, std::experimental::filesystem::, )
+SYMBOL(hard_link_count, std::experimental::filesystem::, )
+SYMBOL(is_block_file, std::experimental::filesystem::, )
+SYMBOL(is_character_file, std::experimental::filesystem::, )
+SYMBOL(is_directory, std::experimental::filesystem::, )
+SYMBOL(is_empty, std::experimental::filesystem::, )
+SYMBOL(is_fifo, std::experimental::filesystem::, )
+SYMBOL(is_other, std::experimental::filesystem::, )
+SYMBOL(is_regular_file, std::experimental::filesystem::, )
+SYMBOL(is_socket, std::experimental::filesystem::, )
+SYMBOL(is_symlink, std::experimental::filesystem::, )
+SYMBOL(last_write_time, std::experimental::filesystem::, )
+SYMBOL(path, std::experimental::filesystem::, )
+SYMBOL(perm_options, std::experimental::filesystem::, )
+SYMBOL(permissions, std::experimental::filesystem::, )
+SYMBOL(perms, std::experimental::filesystem::, )
+SYMBOL(proximate, std::experimental::filesystem::, )
+SYMBOL(read_symlink, std::experimental::filesystem::, )
+SYMBOL(recursive_directory_iterator, std::experimental::filesystem::, )
+SYMBOL(relative, std::experimental::filesystem::, )
+SYMBOL(remove, std::experimental::filesystem::, )
+SYMBOL(remove_all, std::experimental::filesystem::, )
+SYMBOL(rename, std::experimental::filesystem::, )
+SYMBOL(resize_file, std::experimental::filesystem::, )
+SYMBOL(space, std::experimental::filesystem::, )
+SYMBOL(space_info, std::experimental::filesystem::, )
+SYMBOL(status, std::experimental::filesystem::, )
+SYMBOL(status_known, std::experimental::filesystem::, )
+SYMBOL(symlink_status, std::experimental::filesystem::, )
+SYMBOL(temp_directory_path, std::experimental::filesystem::, )
+SYMBOL(u8path, std::experimental::filesystem::, )
+SYMBOL(weakly_canonical, std::experimental::filesystem::, )
+// clang-format on
Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -62,6 +62,23 @@
   EXPECT_EQ("", CI.mapHeader(File));
 }
 
+TEST(CanonicalIncludesTest, CXXSymbolsFromTS) {
+  CanonicalIncludes CI;
+  auto Language = LangOptions();
+  Language.CPlusPlus = true;
+  CI.addSystemHeadersMapping(Language);
+
+  EXPECT_EQ("",
+CI.mapSymbol("std::experimental::filesystem::path"));
+  EXPECT_EQ("",
+CI.mapSymbol("std::experimental::filesystem::file_type"));
+  EXPECT_EQ("",
+CI.mapSymbol("std::experimental::filesystem::file_status"));
+  EXPECT_EQ("",
+CI.mapSymbol(
+"std::experimental::filesystem::recursive_directory_iterator"));
+}
+
 TEST(CanonicalIncludesTest, PathMapping) {
   auto InMemFS = llvm::makeIntrusiveRefCnt();
   FileManager Files(FileSystemOptions(), InMemFS);
Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- 

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

[Younan Zhang via Phabricator via cfe-commits]

zyounan created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
zyounan edited the summary of this revision.
zyounan added reviewers: hokein, sammccall, nridge.
zyounan published this revision for review.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.

Clangd maintains a symbol map from standard library, in order to prevent
unexpected header/symbol leaks from internal files. (e.g. files under
`bits/` for libstdc++) This symbol map was generated by a python script
that parses pages of offline cppreference archive. The script didn't
handle the case for `std::experimental::`, where most symbols are from
TS. It works well as symbols are directly laid out in the corresponding
header under `experimental` directory for most of time.

However, libstdc++'s implementation split symbols of TS FS into a few
header files located in `experimental/bits`. This would make the code
completion provide internal headers when we simply select the symbols.

This fixes https://github.com/clangd/clangd/issues/1481


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142836

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
  clang/include/clang/Tooling/Inclusions/TsStdSymbolMap.inc

Index: clang/include/clang/Tooling/Inclusions/TsStdSymbolMap.inc
===
--- /dev/null
+++ clang/include/clang/Tooling/Inclusions/TsStdSymbolMap.inc
@@ -0,0 +1,55 @@
+// FIXME: Add a parser for experimental pages to `clang/tools/include-mapping/cppreference_parser.py`
+
+SYMBOL(absolute, std::experimental::filesystem::, )
+SYMBOL(canonical, std::experimental::filesystem::, )
+SYMBOL(copy, std::experimental::filesystem::, )
+SYMBOL(copy_file, std::experimental::filesystem::, )
+SYMBOL(copy_options, std::experimental::filesystem::, )
+SYMBOL(copy_symlink, std::experimental::filesystem::, )
+SYMBOL(create_directories, std::experimental::filesystem::, )
+SYMBOL(create_directory, std::experimental::filesystem::, )
+SYMBOL(create_directory_symlink, std::experimental::filesystem::, )
+SYMBOL(create_hard_link, std::experimental::filesystem::, )
+SYMBOL(create_symlink, std::experimental::filesystem::, )
+SYMBOL(current_path, std::experimental::filesystem::, )
+SYMBOL(directory_entry, std::experimental::filesystem::, )
+SYMBOL(directory_iterator, std::experimental::filesystem::, )
+SYMBOL(directory_options, std::experimental::filesystem::, )
+SYMBOL(equivalent, std::experimental::filesystem::, )
+SYMBOL(exists, std::experimental::filesystem::, )
+SYMBOL(file_size, std::experimental::filesystem::, )
+SYMBOL(file_status, std::experimental::filesystem::, )
+SYMBOL(file_time_type, std::experimental::filesystem::, )
+SYMBOL(file_type, std::experimental::filesystem::, )
+SYMBOL(filesystem_error, std::experimental::filesystem::, )
+SYMBOL(hard_link_count, std::experimental::filesystem::, )
+SYMBOL(is_block_file, std::experimental::filesystem::, )
+SYMBOL(is_character_file, std::experimental::filesystem::, )
+SYMBOL(is_directory, std::experimental::filesystem::, )
+SYMBOL(is_empty, std::experimental::filesystem::, )
+SYMBOL(is_fifo, std::experimental::filesystem::, )
+SYMBOL(is_other, std::experimental::filesystem::, )
+SYMBOL(is_regular_file, std::experimental::filesystem::, )
+SYMBOL(is_socket, std::experimental::filesystem::, )
+SYMBOL(is_symlink, std::experimental::filesystem::, )
+SYMBOL(last_write_time, std::experimental::filesystem::, )
+SYMBOL(path, std::experimental::filesystem::, )
+SYMBOL(perm_options, std::experimental::filesystem::, )
+SYMBOL(permissions, std::experimental::filesystem::, )
+SYMBOL(perms, std::experimental::filesystem::, )
+SYMBOL(proximate, std::experimental::filesystem::, )
+SYMBOL(read_symlink, std::experimental::filesystem::, )
+SYMBOL(recursive_directory_iterator, std::experimental::filesystem::, )
+SYMBOL(relative, std::experimental::filesystem::, )
+SYMBOL(remove, std::experimental::filesystem::, )
+SYMBOL(remove_all, std::experimental::filesystem::, )
+SYMBOL(rename, std::experimental::filesystem::, )
+SYMBOL(resize_file, std::experimental::filesystem::, )
+SYMBOL(space, std::experimental::filesystem::, )
+SYMBOL(space_info, std::experimental::filesystem::, )
+SYMBOL(status, std::experimental::filesystem::, )
+SYMBOL(status_known, std::experimental::filesystem::, )
+SYMBOL(symlink_status, std::experimental::filesystem::, )
+SYMBOL(temp_directory_path, std::experimental::filesystem::, )
+SYMBOL(u8path, std::experimental::filesystem::, )
+SYMBOL(weakly_canonical, std::experimental::filesystem::, )
Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -62,6 +62,23 @@