[PATCH] D80535: [ObjectYAML][MachO] Add error handling in MachOEmitter.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/ObjectYAML/MachOEmitter.cpp:30
+inline Error createError(const Twine &Msg) {
+  return createStringError(inconvertibleErrorCode(), Msg);
+}
----------------
Perhaps `invalid_argument` instead of `inconvertibleErrorCode()`?


================
Comment at: llvm/lib/ObjectYAML/MachOEmitter.cpp:290
+              "wrote too much data somewhere, section offsets don't line up");
+        assert(OS.tell() - fileStart <= Sec.offset ||
+               Sec.offset == (uint32_t)0);
----------------
Here and elsewhere, I don't think you need an assertion if you are reporting an error too.


================
Comment at: llvm/test/ObjectYAML/MachO/errors.yaml:1
+## This file contains test cases for error messages in MachOEmitter.
+
----------------
I think these two test cases should each be in their own file, rather than a generic "errors" test file.


================
Comment at: llvm/test/ObjectYAML/MachO/errors.yaml:15
+FatArchs:
+  # Should have 2 entries, but none.
+Slices:
----------------
Perhaps worth having one entry here, to show that zero entries is not specifically the issue here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80535