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

kledzik at apple.com kledzik at apple.com
Thu Jan 2 14:32:39 PST 2014



================
Comment at: lib/ReaderWriter/MachO/MachONormalizedFile.h:126
@@ -119,1 +125,3 @@
+    return X;
+  }
 };
----------------
Joey Gouly wrote:
> 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?
There are other lots of other types in MachONormalized.h that exist for yaml translation (e.g. SectionAttr).  But the compiler is nicer about implicitly casting those types.  

Your latest patch is better in that it only does the copying when going to or from yaml.  But we should be able to do better and not do any copying of section content.

================
Comment at: lib/ReaderWriter/MachO/Atoms.h:25
@@ +24,3 @@
+
+  virtual ContentType contentType() const { return DefinedAtom::typeCode; }
+
----------------
Joey Gouly wrote:
> 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.
Then add a FIXME comment to differentiate temporary code from long term code.


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



More information about the llvm-commits mailing list