ioeric added inline comments.
================
Comment at: clang-query/QueryReplace.cpp:1
+#include "QueryReplace.h"
+#include "QueryParser.h"
----------------
Please add license header.
================
Comment at: clang-query/QueryReplace.cpp:37
+
+llvm::ErrorOr<QueryReplaceSpec> QueryReplaceSpec::parseFromJSON(
+ StringRef Spec) {
----------------
I'd prefer using `llvm::Expected<T>` with an `llvm::StringError`.
================
Comment at: clang-query/QueryReplace.cpp:58
+ Diag.printToStreamFull(llvm::errs());
+ llvm::errs() << "\n";
+ }
----------------
return error here?
Consider using `llvm::Error` with `llvm::StringError`.
================
Comment at: clang-query/QueryReplace.cpp:72
+}
+}
+}
----------------
Missing `// namespace ...`
================
Comment at: clang-query/replace-tool/ClangQueryReplace.cpp:76
+ llvm::ErrorOr<QueryReplaceSpec> Spec =
+ QueryReplaceSpec::parseFromJSON(SpecYaml);
+ if (error_code EC = Spec.getError()) {
----------------
If this parses yaml string, shouldn't it be called `parseFromYAML` instead?
https://reviews.llvm.org/D29622
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
