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

[Michael Spencer]

  This still doesn't address the issue of a file that is being parsed adding a library to the command line directly after it.

  Other than that and inline comments it seems like a good direction.


================
Comment at: include/lld/Core/Error.h:76
@@ +75,3 @@
+
+struct inputGraph_error {
+  enum _ {
----------------
input_graph_error to match existing. We should probably switch them all over to LLVM style later.

================
Comment at: include/lld/Core/LinkingContext.h:314
@@ +313,3 @@
+  /// atoms.
+  virtual void setResolverState(int32_t resolverState) const;
+
----------------
Shankar Kalpathi Easwaran wrote:
> kledzik at apple.com wrote:
> > You can eliminate the ResolveState enum and all the bit values by just making this method be:
> >   void setResolverState(bool addedDefines, bool addedUndefs, bool addedShared, bool addedAbsolute);
> > though I am not sure why anything needs to now what kind of atoms was added.
> > 
> I want to keep the method as is, as its convenient to add new types of atoms that are found.
> 
> The reason I thought it might be useful, was 
> 
> a) Depending on if weak atoms were found in the process, and if the linker flag dictated to resolve weak atoms within the group.
> b) To derive stats of how many atoms were added/resolved from the inputelement (the weight function)
This seems really complicated. Isn't the main purpose of this to know if the resolver added anything? Why not just an argument to nextFile?

================
Comment at: include/lld/Core/LinkingContext.h:360
@@ -355,2 +359,3 @@
   std::unique_ptr<Reader> _yamlReader;
+  std::unique_ptr<Reader> _lldNativeReader;
   StringRefVector _initialUndefinedSymbols;
----------------
_nativeReader?

================
Comment at: include/lld/Core/LinkingContext.h:363-364
@@ -357,3 +362,4 @@
   std::unique_ptr<InputGraph> _inputGraph;
-  llvm::BumpPtrAllocator _allocator;
+  mutable llvm::BumpPtrAllocator _allocator;
+  mutable InputElement *_currentInputElement;
 
----------------
Should we go ahead and make LinkingContext non-const first?

================
Comment at: include/lld/Core/Resolver.h:31-37
@@ -31,5 +30,9 @@
 public:
-  Resolver(const LinkingContext &context, const InputFiles &inputs)
-      : _context(context), _inputFiles(inputs), _symbolTable(context),
-        _result(context), _haveLLVMObjs(false), _addToFinalSection(false),
-        _completedInitialObjectFiles(false) {}
+  enum ResolverState {
+    stateNoChange = 0, // The default resolver state
+    stateNewDefinedAtoms = 1, // New defined atoms were added
+    stateNewUndefinedAtoms = 2, // New undefined atoms were added
+    stateNewSharedLibraryAtoms = 4, // New shared library atoms were added
+    stateNewAbsoluteAtoms = 8 // New absolute atoms were added
+  };
+
----------------
Not llvm style.

================
Comment at: include/lld/Driver/InputGraph.h:247
@@ -212,1 +246,3 @@
+/// directly.
+///
 class FileNode : public InputElement {
----------------
Remove extra empty line.

================
Comment at: lib/Driver/InputGraph.cpp:63-64
@@ +62,4 @@
+    pos = _inputArgs.size();
+  _inputArgs.resize(_inputArgs.size() +
+                    std::distance(inputElements.begin(), inputElements.end()));
+  _inputArgs.insert(_inputArgs.begin() + pos,
----------------
This is unneeded. insert will do the reserve.

================
Comment at: test/Driver/undef-basic.objtxt:22
@@ -21,2 +21,1 @@
 # CHECK:   - name:            undefinedsymbol
-# CHECK:     can-be-null:     at-buildtime
----------------
Why was this removed?

================
Comment at: test/elf/X86_64/dynlib-search.test:2
@@ -1,3 +1,3 @@
 # This tests the functionality for finding the shared library libfn.so for ELF
-RUN: lld -flavor gnu -target x86_64 %p/Inputs/main.o -L%p/Inputs/ -lfn -o %t \
+RUN: lld -flavor gnu -target x86_64 -L%p/Inputs/ -lfn -o %t \
 RUN: --noinhibit-exec -t > %t1
----------------
Why this change?


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