[PATCH] [PassManager] add calls to RoundTrip{YAML, Native} Reader/Writer

[Shankar Kalpathi Easwaran]

================
Comment at: include/lld/Core/PassManager.h:35
@@ -34,3 +34,3 @@
 
-  ErrorOr<void> runOnFile(MutableFile &);
+  ErrorOr<void> runOnFile(std::unique_ptr<MutableFile> &);
 
----------------
Rui Ueyama wrote:
> We don't need to do that in this patch, but ErrorOr<void> looks a bit odd, because it returns nothing  other than an error code. I'd make it to return an llvm::error_code.
Ok.

================
Comment at: include/lld/Core/Resolver.h:7
@@ -6,2 +6,3 @@
 // License. See LICENSE.TXT for details.
+// License. See LICENSE.TXT for details.
 //
----------------
Rui Ueyama wrote:
> duplicate?
will remove.

================
Comment at: include/lld/Core/Resolver.h:67
@@ -64,5 +66,3 @@
 
-  MutableFile& resultFile() {
-    return _result;
-  }
+  std::unique_ptr<MutableFile> resultFile() { return std::move(_result); }
 
----------------
Rui Ueyama wrote:
> To me it does not make sense to release the ownership of _result when resultFile() is called. We can make the LinkingContext object to keep having the ownership. Another merit of not releasing the ownership would be that you wouldn't have to change the function signatures in many places (from MutableFile& to std::unique_ptr<MutableFile>.)
The RoundTripPasses would change the MutableFile to the atoms that are read back from the NativeFile/YAML File.

================
Comment at: include/lld/Passes/RoundTripNativePass.h:10
@@ +9,3 @@
+
+#ifndef LLD_PASSES_ROUNDTRIP_NATIVE_PASS_H
+#define LLD_PASSES_ROUNDTRIP_NATIVE_PASS_H
----------------
Rui Ueyama wrote:
> A rule of the include guard seems inserting an underscore before a capital letter in camel case identifier. So we should s/ROUNDTRIP/ROUND_TRIP/.
ok.

================
Comment at: include/lld/Passes/RoundTripYAMLPass.h:1
@@ +1,2 @@
+//===- Passes/RoundTripYAMLPass.h- Write YAML file/Read it back-----------===//
+//
----------------
Rui Ueyama wrote:
> nit: need space before "-"
ok.

================
Comment at: include/lld/Passes/RoundTripYAMLPass.h:10
@@ +9,3 @@
+
+#ifndef LLD_PASSES_ROUNDTRIP_YAML_PASS_H
+#define LLD_PASSES_ROUNDTRIP_YAML_PASS_H
----------------
Rui Ueyama wrote:
> Ditto
ok.

================
Comment at: include/lld/Passes/RoundTripYAMLPass.h:25
@@ +24,3 @@
+
+  /// Sorts atoms in mergedFile by content type then by command line order.
+  virtual void perform(std::unique_ptr<MutableFile> &mergedFile);
----------------
Rui Ueyama wrote:
> Update the comment.
ok.

================
Comment at: include/lld/ReaderWriter/Simple.h:72
@@ -71,1 +71,3 @@
 
+class FileToMutable : public SimpleFile {
+public:
----------------
Rui Ueyama wrote:
> I guess you added this class to convert a file a mutable file object. I'd think we shouldn't define a new class for that. This can be implemented as a function that is not a member of any class that will create a new instance of SimpleFile from a given File object.
There is no way that you could replace atoms. The FileToMutable class keeps a reference to the old file where the atoms did belong. 

================
Comment at: lib/Passes/RoundTripNativePass.cpp:10
@@ +9,3 @@
+//===----------------------------------------------------------------------===//
+//===----------------------------------------------------------------------===//
+
----------------
Rui Ueyama wrote:
> duplicate
ok.

================
Comment at: lib/Passes/RoundTripYAMLPass.cpp:2
@@ +1,3 @@
+//===- Passes/RoundTripYAMLPass.cpp - Layout atoms
+//-------------------------------===//
+//
----------------
Rui Ueyama wrote:
> Line wrapping
ok.

================
Comment at: lib/Passes/RoundTripYAMLPass.cpp:43
@@ +42,3 @@
+
+  llvm::sys::fs::remove(tmpYAMLFile.str());
+}
----------------
Rui Ueyama wrote:
> Ditto
?


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