[PATCH] D75131: [XCOFF][AIX] Enable -r option for llvm-objdump

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:337
+    report_fatal_error("64-bit support not implemented yet.");
+  const XCOFFSectionHeader32* SectionEntPtr = toSection32(Sec);
+  auto RelocationsOrErr = relocations(*SectionEntPtr);
----------------
clang-format here too. Please check throughout the patch.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:360
+    // relocation offset relative to the start of the section.
+    if (Sec32->VirtualAddress <= RelocAddress && 
+        (Sec32->VirtualAddress + Sec32->SectionSize) > RelocAddress) {
----------------
I believe it is more idiomatic to place the range boundaries on either end of the comparison:
```
Sec32->VirtualAddress <= RelocAddress && RelocAddress < (Sec32->VirtualAddress + Sec32->SectionSize)
```


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:364
+    }
+    Sec32++;
+  }
----------------
I believe it is customary to use prefix increment when possible: https://llvm.org/docs/CodingStandards.html#prefer-preincrement


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:389
+    report_fatal_error("64-bit support not implemented yet.");
+  const XCOFFRelocation32 *Reloc = viewAs<XCOFFRelocation32>(Rel.p);
+  return Reloc->Type;
----------------
Minor nit: Functions that can be "simple one-liners" should be written as such.
The 64-bit part aside, we can have just the return statement.

The overhead of the declaration for `Reloc` is a large proportion of this function, especially considering its (lack of) utility here.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:1
+//===-- XCOFFDump.cpp - XCOFF-specific dumper ---------------------*- C++ -*-===//
+//
----------------
Line exceeds 80 columns.


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

https://reviews.llvm.org/D75131