hokein added inline comments.
================ Comment at: clang/include/clang/Tooling/Inclusions/CXXSymbolMap.inc:1 +//===-- CXXSymbolMap.inc ----------------------------------------*- C++ -*-===// +// ---------------- kadircet wrote: > also maybe rename this to, `AlternativeHeaderMap.inc` ? Renamed it to `StdAlternativeHeaderMap.inc`, to align with the existing `StdSymbolMap.inc` name pattern for C++ symbols. ================ 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>) ---------------- kadircet wrote: > 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>) > ``` Right, thanks for checking it! You're right. I also have checked the C++11, C++17 standard, I don't find any relevant text about the header "<locale>" contains the these symbol definition. I'd rather delete this entry, and only keep `<codecvt>`. ================ 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>) ---------------- kadircet wrote: > 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) Sure. ================ 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, ---------------- kadircet wrote: > 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. Removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143160/new/ https://reviews.llvm.org/D143160 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
