[PATCH] D143160: [include-mapping] Introduce a human-edit CXXSymbolMapping file

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 6 00:25:50 PST 2023


kadircet added inline comments.


================
Comment at: clang/include/clang/Tooling/Inclusions/CXXSymbolMap.inc:12
+SYMBOL(consume_header, std::, <codecvt>) // declared
+SYMBOL(consume_header, std::, <locale>)  // defined
+SYMBOL(generate_header, std::, <codecvt>)
----------------
i feel like cppreference is wrong here (at least per libstdc++-12, and https://eel.is/c++draft/locale.syn#header:%3clocale%3e)

```
#include <locale>
auto x = std::consume_header;
```

same for generate_header and little_endian.

it might be worth changing these into:
```
SYMBOL(consume_header, std::, <codecvt>)
// cppreference mentions this header as an alternative, but it isn't in practice.
// SYMBOL(consume_header, std::, <locale>) 
```


================
Comment at: clang/include/clang/Tooling/Inclusions/CXXSymbolMap.inc:19
+SYMBOL(mbstate_t, std::, <cuchar>)
+SYMBOL(size_t, std::, <cstddef>)
+SYMBOL(size_t, std::, <cstdio>)
----------------
can we actually use the ordering from standard: `[cstddef.syn], [cstdlib.syn], [cstring.syn], [cwchar.syn], [cuchar.syn], [ctime.syn], [cstdio.syn]` [(1)](https://eel.is/c++draft/libraryindex#:~:text=size_%C2%ADt%2C%20%5Bexpr.sizeof%5D%2C%20%5Bcstddef.syn%5D%2C%20%5Bcstdlib.syn%5D%2C%20%5Bcstring.syn%5D%2C%20%5Bcwchar.syn%5D%2C%20%5Bcuchar.syn%5D%2C%20%5Bctime.syn%5D%2C%20%5Bcstdio.syn%5D)


================
Comment at: clang/include/clang/Tooling/Inclusions/CXXSymbolMap.inc:26
+SYMBOL(size_t, std::, <cwchar>)
+// C++ [meta.trans.other 21.3.8.7]:
+// In addition to being available via inclusion of the <type_­traits> header,
----------------
nit: i don't think we need the comments here, both headers are also mentioned in cppreference pages.
i am afraid these might get out of date (or only seldom mentioned) as future editors of this mapping might not be delicate enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143160



More information about the cfe-commits mailing list