[PATCH] [lld] add InputGraph to process Command line arguments

[Shankar Kalpathi Easwaran]

  I will remove the Option class, I will move the ELFGroup class as a Group class derived from Control.


================
Comment at: include/lld/Core/TargetInfo.h:240
@@ +239,3 @@
+  void appendLLVMOption(const char *opt) { _llvmOptions.push_back(opt); }
+  virtual void setInputGraph(std::unique_ptr<InputGraph> &inputGraph) {
+    _inputGraph = std::move(inputGraph);
----------------
Michael Spencer wrote:
> This should not be a reference. Just pass by value.
ok.

================
Comment at: include/lld/Core/TargetInfo.h:243
@@ +242,3 @@
+  }
+  virtual const std::unique_ptr<InputGraph> &inputGraph() const {
+    return _inputGraph;
----------------
Michael Spencer wrote:
> This should not return a unique_ptr. Just an InputGraph&.
ok.

================
Comment at: include/lld/Driver/Driver.h:41
@@ -38,3 +40,3 @@
 
-  /// Performs link using specified options.
+  /// Performs link using specified options
   static bool link(const TargetInfo &targetInfo,
----------------
Michael Spencer wrote:
> Comments in LLVM contain full sentences and end with a period.
ok.

================
Comment at: include/lld/Driver/GnuLDInputGraph.h:67-69
@@ +66,5 @@
+    std::unique_ptr<ELFLinkerOptions> linkerOptions(new ELFLinkerOptions());
+    linkerOptions.get()->setForceLoad(_forceLoad);
+    linkerOptions.get()->setAsNeeded(_asNeeded);
+    linkerOptions.get()->copyLibraryPaths(_libraryPaths);
+    return std::move(linkerOptions);
----------------
Michael Spencer wrote:
> Why .get()? unique_ptr has an overloaded operator ->.
ah right.

================
Comment at: include/lld/Driver/GnuLDInputGraph.h:95
@@ +94,3 @@
+
+  static inline bool classof(ELFLinkerOptions *) { return true; }
+
----------------
Michael Spencer wrote:
> Not needed.
ok.

================
Comment at: include/lld/Driver/GnuLDInputGraph.h:124
@@ +123,3 @@
+
+  static inline bool classof(const ELFFileNode *) { return true; }
+
----------------
Michael Spencer wrote:
> Not needed.
ok.

================
Comment at: include/lld/Driver/GnuLDInputGraph.h:152
@@ +151,3 @@
+
+  static inline bool classof(const ELFControlNode *) { return true; }
+
----------------
Michael Spencer wrote:
> Not needed.
ok.

================
Comment at: include/lld/Driver/GnuLDInputGraph.h:173
@@ +172,3 @@
+
+  static inline bool classof(const ELFGroup *) { return true; }
+
----------------
Michael Spencer wrote:
> Not needed.
ok.

================
Comment at: include/lld/Driver/InputGraph.h:123
@@ +122,3 @@
+  /// Return the Element Type for an Input Element
+  virtual ElementType type() const { return _type; }
+
----------------
Michael Spencer wrote:
> This is generally referred to as Kind in LLVM.
ok.

================
Comment at: include/lld/Driver/InputGraph.h:167
@@ +166,3 @@
+
+  static inline bool classof(const Option *) { return true; }
+};
----------------
Michael Spencer wrote:
> Not needed.
ok.

================
Comment at: include/lld/Driver/InputGraph.h:192
@@ +191,3 @@
+
+  static inline bool classof(const ControlNode *) { return true; }
+
----------------
Michael Spencer wrote:
> Not needed.
ok.

================
Comment at: include/lld/Driver/InputGraph.h:197
@@ +196,3 @@
+
+  /// \brief Iterators to iterate the
+  InputGraph::InputElementIterT begin() { return _elements.begin(); }
----------------
Michael Spencer wrote:
> Incomplete sentence.
will fix.

================
Comment at: include/lld/Driver/InputGraph.h:228
@@ +227,3 @@
+
+  static inline bool classof(const FileNode *) { return true; }
+
----------------
Michael Spencer wrote:
> Not needed.
ok.

================
Comment at: lib/Driver/GnuLdDriver.cpp:350
@@ +349,3 @@
+      std::unique_ptr<InputElement> simpleControl =
+          std::move(gnuLinkerOptions.getInputElement(*inputArg, optionType));
+      if (controlNodeStack.empty())
----------------
Michael Spencer wrote:
> This doesn't need a move. Return values are already rvalues.
ok.

================
Comment at: lib/Driver/GnuLdDriver.cpp:354
@@ +353,3 @@
+      else
+        ((llvm::dyn_cast<ControlNode>)(controlNodeStack.top()))
+            ->processInputElement(std::move(simpleControl));
----------------
Michael Spencer wrote:
> This doesn't need the extra parens.
ok.


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