[PATCH] [lld] Registry model for Readers and Reference Kind strings

[kledzik at apple.com]

================
Comment at: include/lld/Core/LinkingContext.h:280
@@ -285,9 +279,3 @@
 
-  /// Returns the YAML reader.
-  virtual Reader &getYAMLReader() const { return *_yamlReader; }
-
-  /// Returns the LLD Native file format reader.
-  virtual Reader &getNativeReader() const { return *_nativeReader; }
-
-  /// Return the default reader for the target
-  virtual Reader &getDefaultReader() const = 0;
+  /// 
+  const Registry &registry() const { return _registry; }
----------------
Shankar Kalpathi Easwaran wrote:
> empty comment ?
fixed

================
Comment at: include/lld/Core/LinkingContext.h:281-282
@@ -294,1 +280,4 @@
+  /// 
+  const Registry &registry() const { return _registry; }
+  Registry &registry() { return _registry; }
 
----------------
Shankar Kalpathi Easwaran wrote:
> Isnt having two functions returning the same in const/ non-const mode confusing ? We might want to settle on whether we want to use a const vs. non-const ?
This is the standard way to allow non-const LinkingContexts to get a non-const registry and limit const LinkingContexts to only get a const registry. 

================
Comment at: include/lld/Core/Reference.h:42-43
@@ +41,4 @@
+  enum class KindNamespace { 
+    all     = 0,
+    testing = 1,
+    ELF     = 2,
----------------
Shankar Kalpathi Easwaran wrote:
> What is all, testing ? is this for the core flavor ?
"all" is used with values like "kindInGroup" which are applicable to all clients. 
"testing" is for use with the test cases the use the CoreDriver.  

================
Comment at: include/lld/Driver/DarwinInputGraph.h:56
@@ -54,6 +55,3 @@
 
-    if (filePath->endswith(".objtxt"))
-      return ctx.getYAMLReader().parseFile(_buffer, _files);
-
-    (void) (_isWholeArchive);
-    return error_code::success();
+    if (_isWholeArchive) {
+      std::vector<std::unique_ptr<File>> parsedFiles;
----------------
Joey Gouly wrote:
> Should this change be included?
When mach-o starts handling archives, it will need this for use with the -all_load option.

================
Comment at: include/lld/ReaderWriter/ELFLinkingContext.h:235
@@ -230,3 +234,3 @@
 protected:
-  ELFLinkingContext(llvm::Triple, std::unique_ptr<TargetHandlerBase>);
+  ELFLinkingContext( llvm::Triple, std::unique_ptr<TargetHandlerBase>);
 
----------------
Joey Gouly wrote:
> Randomly inserted whitespace!
fixed

================
Comment at: include/lld/ReaderWriter/FileArchive.h:15-20
@@ -14,8 +14,8 @@
 
 #include "llvm/Object/Archive.h"
 #include "llvm/Support/MemoryBuffer.h"
 
 #include <unordered_map>
 
 namespace lld {
 
----------------
Shankar Kalpathi Easwaran wrote:
> we should delete this file, as its not providing any functionality.
I thought I had "svn rm"ed it, but apparently not.  I've now removed it.

================
Comment at: include/lld/ReaderWriter/Reader.h:87
@@ +86,3 @@
+  // method is used.  Any options that a Reader might need must be passed
+  // as parameters to the adSupport*() method.
+  void addSupportArchives(bool logLoading);
----------------
Shankar Kalpathi Easwaran wrote:
> nitpick : adSupport -> addSupport.
fixed

================
Comment at: include/lld/ReaderWriter/Reader.h:81
@@ +80,3 @@
+  bool referenceKindToString(Reference::KindNamespace ns, Reference::KindArch a, 
+                             uint16_t value, StringRef &) const;
+  
----------------
Joey Gouly wrote:
> I'm wondering if we should use a typedef for Reference::KindValue?
Good Idea.  I've added that typedef and changed lots of places to use it.

================
Comment at: include/lld/ReaderWriter/YamlContext.h:20
@@ +19,3 @@
+namespace lld {
+  class File;
+  class LinkingContext;
----------------
Joey Gouly wrote:
> Indentation looks really weird here.
The innermost namespace was not indented properly.  Fixed.

================
Comment at: lib/ReaderWriter/ELF/Atoms.h:803
@@ +802,3 @@
+  
+  void addReferenceELF_Hexeagon(uint16_t relocType, uint64_t off, const Atom *t,
+                                Reference::Addend a) {
----------------
Shankar Kalpathi Easwaran wrote:
> Hexeagon->Hexagon
Fixed


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

BRANCH
  svn

ARCANIST PROJECT
  lld