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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to