[PATCH] D67445: [yaml2obj/ObjectYAML] - Cleanup the error reporting API, add custom errors handlers.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 11 08:16:52 PDT 2019


jhenderson added a comment.

Broadly looks fine to me.



================
Comment at: include/llvm/ObjectYAML/yaml2obj.h:47
 
-int yaml2coff(COFFYAML::Object &Doc, raw_ostream &Out);
-int yaml2elf(ELFYAML::Object &Doc, raw_ostream &Out);
-int yaml2macho(YamlObjectFile &Doc, raw_ostream &Out);
-int yaml2minidump(MinidumpYAML::Object &Doc, raw_ostream &Out);
-int yaml2wasm(WasmYAML::Object &Doc, raw_ostream &Out);
+typedef llvm::function_ref<void(const Twine &Msg)> ErrorHandler;
 
----------------
using instead of typedef maybe?


================
Comment at: lib/ObjectYAML/MachOEmitter.cpp:18
 #include "llvm/ObjectYAML/yaml2obj.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/LEB128.h"
----------------
I haven't yet looked at the other files, but this seems like an unnecessary include, right?


================
Comment at: lib/ObjectYAML/WasmEmitter.cpp:130
+
+void WasmWriter::writeInitExpr(raw_ostream &OS, const wasm::WasmInitExpr &InitExpr) {
   writeUint8(OS, InitExpr.Opcode);
----------------
clang-format?


================
Comment at: unittests/ObjectYAML/YAML2ObjTest.cpp:60
+
+  // 1. Test the yaml2ObjectFile().
+
----------------
Delete "the"


================
Comment at: unittests/ObjectYAML/YAML2ObjTest.cpp:69
+
+  // 2. Test the convertYAML(). 
+
----------------
Delete "the"


================
Comment at: unittests/ObjectYAML/YAML2ObjTest.cpp:82
+}
\ No newline at end of file

----------------
No newline at end of file


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

https://reviews.llvm.org/D67445





More information about the llvm-commits mailing list