hokein added a comment.
The implementation looks good, but to see whether sam has high-level comments
on this.
================
Comment at: clangd/index/SymbolYAML.cpp:140
llvm::yaml::Output Yout(OS);
for (Symbol S : Symbols) // copy: Yout<< requires mutability.
Yout<< S;
----------------
The function could be simplified by using the SymbolToYAML below.
```
std::string Str;
for (const Symbol& S : Symbols) {
Str += SymbolToYAML(S);
}
```
================
Comment at: clangd/index/SymbolYAML.cpp:145
+std::string SymbolToYAML(const Symbol& Sym) {
+ std::string Str;
----------------
might be use `Symbol` as parameter type.
================
Comment at: clangd/index/SymbolYAML.h:30
+// Convert a single symbol to YAML-format string.
+std::string SymbolToYAML(const Symbol &Sym);
----------------
might be also mention `The YAML result is safe to concatenate`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41730
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
