[PATCH] D88749: [SystemZ/z/OS] Add GOFF reader

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

You may consider moving llvm-objdump to a separate patch. Some parts should be tested by a unittest in unittest/Object/



================
Comment at: llvm/include/llvm/BinaryFormat/GOFF.h:30
+
+const uint8_t RecordLength = 80;
+
----------------
const -> constexpr


================
Comment at: llvm/lib/Object/Binary.cpp:70
   case file_magic::coff_object:
+  case file_magic::goff_object:
   case file_magic::coff_import_library:
----------------
Don't break the continuation of coff_* file types.


================
Comment at: llvm/lib/Object/GOFFObjectFile.cpp:260
+  if (BindingStrength == GOFF::ESD_BST_Weak) {
+    Flags |= SymbolRef::SF_Weak;
+  }
----------------
http://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

This applies to  many other places


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88749