[PATCH] D98437: [SystemZ][z/OS] Add GOFFObjectFile class support for HDR, ESD and END records

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 15 23:48:13 PDT 2021


MaskRay added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/GOFF.h:25
+
+constexpr uint8_t RecordLength = 80;
+const uint8_t RecordPrefixLength = 3;
----------------
inconsistent constexpr/const. You can use constexpr everywhere.


================
Comment at: llvm/include/llvm/Object/GOFF.h:25
+namespace llvm {
+
+namespace object {
----------------
the prevailing code style doesn't insert a blank line between nested namespaces


================
Comment at: llvm/include/llvm/Object/GOFF.h:82
+public:
+  HDRRecord() {}
+
----------------
delete user-defined constructor  which does nothing. ditto elsewhere in the file


================
Comment at: llvm/lib/Object/GOFFObjectFile.cpp:25
+using namespace llvm;
+using namespace object;
+
----------------
llvm::object


================
Comment at: llvm/lib/Object/GOFFObjectFile.cpp:338
+
+  for (size_t I = 0, E = SectionList.size(); I < E; ++I) {
+    bool Found;
----------------
`!=` is more common


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98437



More information about the llvm-commits mailing list