[PATCH] D63270: [clangd] Add include-mapping for C symbols.

[Haojian Wu via Phabricator via cfe-commits]

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