[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clangd/StdGen/StdGen.py:2
+#!/usr/bin/env python
+#===- StdGen.py -  -------------------------------------------*- python -*--===#
+#
----------------
ioeric wrote:
> I'd avoid abbreviation in the file name and the new directory name. `StdGen` sounds a lot like a library ;)
> 
> I'd suggest something like `std-include-mapping/generate.py`
Personally I'd vote `StdGen`, and it follows llvm's `TableGen`.  The name `std-include-mapping/generate.py` seems too verbose...


================
Comment at: clangd/StdGen/StdGen.py:32
+
+STDGEN_CODE = """\
+//===-- StdGen'erated file --------------------------------------*- C++ -*-===//
----------------
ioeric wrote:
> nit: maybe `STDGEN_CODE_HEADER`?
renamed to `STDGEN_CODE_PREFIX`.


================
Comment at: clangd/StdGen/StdGen.py:94
+  cpp_symbol_root = os.path.join(cpp_reference_root, "en", "cpp")
+  if not os.path.exists(cpp_reference_root):
+    exit("Path %s doesn't exist!" % cpp_reference_root)
----------------
ioeric wrote:
> why not check `exists(index_page_path)` instead?
I think either way works.


================
Comment at: clangd/StdSymbolMap.inc:8
+//===----------------------------------------------------------------------===//
+// Used to build a lookup table (qualified names => include headers) for C++
+// Standard Library symbols.
----------------
ioeric wrote:
> can we include the information about the HTML archive (e.g. STL version) from which this table is generated? This would be useful for future maintenance.
The version of HTML archive is the date information, but this information is only present in the `.zip` name, we lose it after we unzip the `zip` file....


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58345