[PATCH] [lld] enable mach-o and native yaml to be intermixed

[Rui Ueyama]

  Overall looking good. A few comments.


================
Comment at: include/lld/ReaderWriter/Reader.h:25
@@ +24,3 @@
+namespace llvm {
+  namespace yaml {
+    class IO;
----------------
Don't indent namespaces.

================
Comment at: lib/ReaderWriter/MachO/File.h:24
@@ -23,2 +23,3 @@
 
-  void addDefinedAtom(StringRef name, ArrayRef<uint8_t> content) {
+  void addDefinedAtom(StringRef name, ArrayRef<uint8_t> content, bool copyRefs){
+    if (copyRefs) {
----------------
nit: need a space before "{"

================
Comment at: lib/ReaderWriter/MachO/File.h:29
@@ +28,3 @@
+      memcpy(s, name.data(), name.size());
+      name = StringRef(s, name.size());
+      uint8_t *bytes = _allocator.Allocate<uint8_t>(content.size());
----------------
Does name = *new (_allocator) std::string(name.str()) work?

================
Comment at: lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp:628
@@ +627,3 @@
+                                                 const lld::File *&file) const {
+  if (io.mapTag("!mach-o")) {
+    // Step 1: parse yaml into normalized mach-o struct.
----------------
I'd invert the expression: if (!io.mapTag("!mach-o")) return false; for early return.

================
Comment at: lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp:640
@@ +639,3 @@
+    if (foe) {
+      std::unique_ptr<lld::File> f = std::move(*foe);
+      file = f.release();
----------------
Rafael recently added get method to ErrorOr, which I think is preferred way to get a value of ErrorOr, so please use it instead of *.

================
Comment at: lib/ReaderWriter/YAML/ReaderWriterYAML.cpp:671
@@ +670,3 @@
+    if (info->_registry && info->_registry->handleTaggedDoc(io, file))
+        return;
+    // If no registered handler claims this tag and there is no tag, 
----------------
Wrong indentation.

================
Comment at: lib/ReaderWriter/YAML/ReaderWriterYAML.cpp:674
@@ -673,1 +673,3 @@
+    // grandfather in as "!native".
+    if (io.mapTag("!native", true) || io.mapTag("tag:yaml.org,2002:map"))
       mappingAtoms(io, file);
----------------
What is this "tag:yaml.org,2002:map"?


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

BRANCH
  svn

ARCANIST PROJECT
  lld