[PATCH] Refactor parseFile API to use LinkerInput directly

Rui Ueyama ruiu at google.com
Wed Sep 4 10:35:07 PDT 2013


  Looks good to me. Passing both file contents and a object representing a file (except contents) around did not make much sense, so it simplified the API.

  As to Shankar's concerns, I don't think this patch increased the complexity of LinkerInput class. This patch even didn't add any new field to the class. As to possible subclassing of LinkerInput, if we need such class, we should define a new parent class of LinkerInput and make the new subclass and LinkerInput to inherit from the new parent class. We don't really have to define a subclass of LinkerInput but can change the inheritance hierarchy.


================
Comment at: lib/ReaderWriter/Reader.cpp:29
@@ -28,3 +20,1 @@
-  return this->parseFile(mb, result);
-}
 } // end namespace lld
----------------
I think we still need this method. It may not have to be in this class, but you inline-expanded this method into each driver, resulting repeating code duplication. These really need to be consolidated.

================
Comment at: lib/Driver/DarwinLdDriver.cpp:86
@@ -77,1 +85,3 @@
+
+  return std::unique_ptr<LinkerInput>(new LinkerInput(std::move(mb), *filePath));
 }
----------------
We shouldn't have duplicate code in each driver.


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



More information about the llvm-commits mailing list