hokein added a comment.
Thanks for the comments!
================
Comment at: clangd/index/SymbolFromYAML.cpp:32
+// supported in YAML I/O.
+struct NPArray {
+ NPArray(IO &) {}
----------------
sammccall wrote:
> what does NP stand for?
Ah, P is a typo here. N stands for Normalized. Renamed it to `NormalizedXXX`.
================
Comment at: clangd/index/SymbolFromYAML.cpp:44
+
+ std::vector<uint8_t> Value;
+};
----------------
sammccall wrote:
> Nit: for readability, I suggest you encode as a hex string instead.
>
> We don't yet have operator<< for SymbolID, but I think we should, and it
> should be consistent.
> So preferably define `operator<<` for SymbolID, and use it here, rather than
> just defining everything here.
Good idea. Done.
================
Comment at: clangd/index/SymbolFromYAML.h:1
+//===--- SymbolFromYAML.h ----------------------------------------*-
C++-*-===//
+//
----------------
sammccall wrote:
> nit: `SymbolFromYAML.h` seems slightly off for the file, because you support
> both conversions (and also multiple symbols)
>
> Consider just `YAML.h`?
`YAML` seems too general. Named it `SymbolYAML` indicating the conversion
between Symbol and YAML.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41178
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
