[PATCH] D89071: [SystemZ/z/OS] Add GOFFObjectFile class and details of GOFF file format

[Yusra Syeda via Phabricator via llvm-commits]

yusra.syeda marked 5 inline comments as done.
yusra.syeda added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/GOFF.h:53
+
+enum ESDSymbolType {
+  ESD_ST_SectionDefinition = 0,
----------------
jhenderson wrote:
> Does the GOFF spec specify the size of symbol types and other things like that? If so, I'd use the explicit size for these fields. For example, if symbol type is guaranteed to be a single byte, you might adopt my inline edit. The same goes for each other enum below.
I've added the sizes where applicable.


================
Comment at: llvm/lib/Object/GOFFObjectFile.cpp:136
+    case GOFF::RT_LEN:
+      LLVM_DEBUG(dbgs() << "  --  LEN (GOFF record type) unhandled\n");
+      break;
----------------
jhenderson wrote:
> jhenderson wrote:
> > This sounds like it should be an error?
> I think this comment was referring to the "unhandled" bits below. It's been marked as done, but I don't see any response. Could you clarify more why this isn't a hard error and instead such things are being ignored?
These should not be errors. The GOFF reader ignores the normally less important things. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89071