[PATCH] [lld][inputGraph] Implement flavor specific resolver

Rui Ueyama ruiu at google.com
Mon Oct 7 21:18:27 PDT 2013



================
Comment at: include/lld/Driver/InputGraph.h:330
@@ +329,3 @@
+/// \brief Represents Internal Input files
+class SimpleFileNode : public InputElement {
+public:
----------------
Shankar Kalpathi Easwaran wrote:
> Rui Ueyama wrote:
> > Shankar Kalpathi Easwaran wrote:
> > > Rui Ueyama wrote:
> > > > Does SimpleFileNode contains multiple files? The name is confusing.
> > > It resembles SimpleFile. It does contain multiple files, but is usually a place holder for files that are internally added by the Linker. See Driver. 
> > > 
> > I'd name it SimpleArchiveFileNode.
> Its not intended for Archives. Its intended for any internal file that you want to add. For example ImplicitFiles/InternalFiles.
A file containing multiple files is usually called an archive no? It may or may not correspond to an .a file on disk, but it doesn't matter.

================
Comment at: include/lld/Driver/InputGraph.h:59
@@ +58,3 @@
+    END
+  };
+
----------------
Shankar Kalpathi Easwaran wrote:
> Rui Ueyama wrote:
> > Shankar Kalpathi Easwaran wrote:
> > > Rui Ueyama wrote:
> > > > I feel like InputGraph is overly generalized. It's generality reflected the need that Resolver have to handle input files in a platform-neutral manner, and input files can be complicated because of --{start,end}-group, -z something, etc. Now these complexities are hidden behind each flavor's getNextFile() method. That would give us a chance to reduce its complexity by representing input files using simpler data structure.
> > > > 
> > > > This is not a review comment that needs to be resolved soon though.
> > > It has to be a common infrastructure, that is used over all flavors. Its important to have these functions to insert input elements at any place in the graph. 
> > My point is the way to abstract linker input as InputGraph and InputElement itself can be simplified.
> Isnt the current model simplistic ?
Honestly, no, I feel that it's over engineerded a bit and designed to be too generic. It may have been unavoidable with the previous design but we may be able to simplify it with nextFile() infrastructure.


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



More information about the llvm-commits mailing list