hokein added inline comments.
================ Comment at: clang-tools-extra/clangd/include-mapping/gen_std.py:90 + symbol_index_root = page_root + parse_pages = [(page_root, "index.html", "")] + ---------------- kadircet wrote: > maybe we should rather pass some something like "INVALID" as namespace for C > symbols? changed to `None` ================ Comment at: clang-tools-extra/clangd/include-mapping/gen_std.py:95 - parse_pages = [ - (cpp_root, "symbol_index.html", "std::"), - # std sub-namespace symbols have separated pages. - # We don't index std literal operators (e.g. - # std::literals::chrono_literals::operator""d), these symbols can't be - # accessed by std::<symbol_name>. - # FIXME: index std::placeholders symbols, placeholders.html page is - # different (which contains one entry for _1, _2, ..., _N), we need special - # handling. - (symbol_index_root, "chrono.html", "std::chrono::"), - (symbol_index_root, "filesystem.html", "std::filesystem::"), - (symbol_index_root, "pmr.html", "std::pmr::"), - (symbol_index_root, "regex_constants.html", "std::regex_constants::"), - (symbol_index_root, "this_thread.html", "std::this_thread::"), - ] - - symbols = [] - # Run many workers to process individual symbol pages under the symbol index. - # Don't allow workers to capture Ctrl-C. - pool = multiprocessing.Pool( - initializer=lambda: signal.signal(signal.SIGINT, signal.SIG_IGN)) - try: - for root_dir, page_name, namespace in parse_pages: - symbols.extend(GetSymbols(pool, root_dir, page_name, namespace)) - finally: - pool.terminate() - pool.join() + symbols = cppreference_parser.GetSymbols(parse_pages) ---------------- kadircet wrote: > I believe it is more sensible for this function to take `(args.cppreference, > args.language)` and perform the above mentioned logic to generate > `parse_pages` from these two internally. Moving the above logic code to cppreference doesn't simplify the code, I'd prefer to keep the `cppreference.py` as simple as possible; and we also use the `page_root` after calling this function (line 99). ================ Comment at: clang-tools-extra/clangd/include-mapping/test.py:1 #!/usr/bin/env python #===- test.py - ---------------------------------------------*- python -*--===# ---------------- kadircet wrote: > no new tests for c symbol parsing ? not needed, as we share the same parsing logic and we don't introduce new behavior in this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63270/new/ https://reviews.llvm.org/D63270 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
