[PATCH] D59419: [XCOFF] Add functionality for parsing AIX XCOFF object files header .

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

The skeleton of the dumper seems fine.

My main question would be, how is this going to fit into MC and the rest of LLVM? We support 4(?) object file formats there now (ELF, COFF (pretty much assumes Windows), wasm, and MachO), and MC is not designed to be very extensible. Will Triple::isOSBinFormatCOFF return true for XCOFF, or will it be distinct?



================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:36
+
+struct XCOFFFileHeader {
+  support::ubig16_t Magic;
----------------
sfertile wrote:
> Rather then defining these in the `llvm::object` namespace why not drop the `XCOFF` part of the name and make the defintion nested inside `XCOFFObjectFile`?
(Not to bikeshed too much...) Personally, I'd leave this as is. It's consistent with the other object formats. It's useful to be able to do `using namespace llvm::object;`, but you cannot shorten `XCOFFObjectFile::Header` without a typedef or type alias.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59419