hokein added a comment. In D58345#1401907 <https://reviews.llvm.org/D58345#1401907>, @sammccall wrote:
> In D58345#1401213 <https://reviews.llvm.org/D58345#1401213>, @hokein wrote: > > > In D58345#1401040 <https://reviews.llvm.org/D58345#1401040>, @ioeric wrote: > > > > > Looks great! Thanks for doing this. > > > > > > Could you also check in the tool that is used to generate the mapping? We > > > need a way to update the mapping when cppreference is updated. > > > > > > The tool is not ready yet, I'm still polishing it, but the results are > > good, I think this patch should not be blocked by the tool. > > > Generated files aren't really source code, I agree with Eric we should check > in the tool with this patch. Added the tool in this patch, but I think a separate repository under GitHub's clangd org might be a more suitable place. For simplicity, I re-scoped this patch -- only emits unique symbols. We have ideas to handle ambiguous symbols, will address them in a followup. ================ Comment at: clangd/StdSymbolMap.imp:410 +{ "std::gslice_array", { "<valarray>" } }, +{ "std::hardware_constructive_interference_size", { "<new>" } }, // C++11 +{ "std::hardware_destructive_interference_size", { "<new>" } }, // C++11 ---------------- sammccall wrote: > I'm not sure if the language comments are useful to human readers, who can > just look this stuff up. If you think they might be useful to code, we could > include them in tablegen-style macros. > > In any case, this is a C++17 symbol, so maybe something's wrong with the > generator. > > In any case, this is a C++17 symbol, so maybe something's wrong with the > generator. hmm, it is a bug in cppreference index page, this symbol is wrongly marked C++11. ================ Comment at: clangd/StdSymbolMap.imp:1283 +{ "std::log10", { "<cmath>", "<complex>", "<valarray>" } }, +{ "std::make_error_code", { "<system_error>", "<ios>" } }, // C++11 +{ "std::make_error_condition", { "<system_error>", "<ios>" } }, // C++11 ---------------- sammccall wrote: > we're missing the `future` variant I think? looks like it was a bug in the generator code, fixed. ================ Comment at: clangd/StdSymbolMap.imp:1291 +{ "std::sinh", { "<cmath>", "<complex>", "<valarray>" } }, +{ "std::size_t", { "<cwchar>", "<ctime>", "<cstring>", "<cstdlib>", "<cstdio>", "<cstddef>" } }, +{ "std::sqrt", { "<cmath>", "<complex>", "<valarray>" } }, ---------------- sammccall wrote: > this is interesting: any of the options are valid. IMO the best one here is > `<cstddef>`. > But we should be picking by policy, not by looking at the header where the > symbol is actually defined. We could do this on either the generator or > consumer side. this can be addressed by using a whitelist in the generator. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58345/new/ https://reviews.llvm.org/D58345 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
