[PATCH] [lld] [MachO] Begin to flesh out normalizedToAtoms

[Joey Gouly]

  Thanks for the quick review!


================
Comment at: lib/ReaderWriter/MachO/MachONormalizedFile.h:126
@@ -119,1 +125,3 @@
+    return X;
+  }
 };
----------------
kledzik at apple.com wrote:
> We really don't want to be copying around section content.  This should just return an ArrayRef to the section content.
Currently ContentType is a vector of Hex8 (which is a YAML strong typedef)., so I'd have to return ArrayRef<Hex8> rather than ArrayRef<uint8_t>. To me it seems weird to use the YAML::Hex8 in places other than the YAML part.

I could change ContentBytes to be uint8_t, and provide a method getContentHex(), to use in MachONormalizedFileYAML.cpp?

Is there another / better way?

================
Comment at: lib/ReaderWriter/MachO/Atoms.h:25
@@ +24,3 @@
+
+  virtual ContentType contentType() const { return DefinedAtom::typeCode; }
+
----------------
kledzik at apple.com wrote:
> All symbol based atoms will not be typeCode.  There are DefinedAtoms without content (zero fill).  There are content atoms without symbols (c-string literals).  So, we are going to need a bunch of atom classes.
Yeah, I was just keeping it simple for now.

================
Comment at: unittests/MachOTests/MachONormalizedFileToAtomsTests.cpp:71
@@ +70,3 @@
+  };
+  std::unique_ptr<NormalizedFile> f = fromBinary(fileBytes, sizeof(fileBytes));
+  ErrorOr<std::unique_ptr<const lld::File>> atom_f = normalizedToAtoms(*f, "");
----------------
kledzik at apple.com wrote:
> This test case is supposed to test "normalized -> atoms".  There is no need for binary here.  Use code to stuff fields in a NormalizedMachOFile and then call normalizedObjectToAtoms() on it.
Ok, I will rewrite this. These tests could become quite cumbersome, so as soon as we can go via the driver, it'll be better.


http://llvm-reviews.chandlerc.com/D2464