[PATCH] [lld] add calls to nativereader/nativewriter

kledzik at apple.com kledzik at apple.com
Wed Oct 23 17:14:30 PDT 2013


  I'm curious about the failures this uncovered.  What info was lost through the new passes?


================
Comment at: lib/Passes/RoundTripYAMLPass.cpp:24
@@ +23,3 @@
+/// Perform the actual pass
+void RoundTripYAMLPass::perform(std::unique_ptr<MutableFile> &yamlFile) {
+  ScopedTask task(getDefaultDomain(), "RoundTripYAMLPass");
----------------
other passes call the parameter "mergedFile".  Calling it yamlFile is misleading

================
Comment at: lib/Passes/RoundTripYAMLPass.cpp:26
@@ +25,3 @@
+  ScopedTask task(getDefaultDomain(), "RoundTripYAMLPass");
+  std::unique_ptr<Writer> _yamlWriter = createWriterYAML(_context);
+  SmallString<128> tmpYAMLFile;
----------------
_yamlWriter is a local variable, so should not have a leading underscore.

================
Comment at: include/lld/Passes/RoundTripYAMLPass.h:33
@@ +32,3 @@
+  LinkingContext &_context;
+  std::vector<std::unique_ptr<File> > _yamlFile;
+};
----------------
_yamlFile seems like it should be a local variable in perform().  But I bet it is an ivar to keep it alive after the pass is done, because it actally owns the atoms that will be used for the rest of the link.  At least add a comment explaining that.

================
Comment at: lib/Passes/RoundTripNativePass.cpp:25
@@ +24,3 @@
+void RoundTripNativePass::perform(std::unique_ptr<MutableFile> &nativeFile) {
+  ScopedTask task(getDefaultDomain(), "RoundTripNativePass");
+  std::unique_ptr<Writer> _nativeWriter = createWriterNative(_context);
----------------
other passes call the parameter "mergedFile". Calling it nativeFile is misleading.

================
Comment at: lib/Passes/RoundTripNativePass.cpp:27
@@ +26,3 @@
+  std::unique_ptr<Writer> _nativeWriter = createWriterNative(_context);
+  SmallString<128> tmpNativeFile;
+  // Separate the directory from the filename
----------------
_nativeWriter is a local variable, so should not have a leading underscore.


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



More information about the llvm-commits mailing list