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

[Shankar Kalpathi Easwaran]

  I like this change, very flexible.


================
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; }
----------------
empty comment ?

================
Comment at: include/lld/Core/LinkingContext.h:281-282
@@ -294,1 +280,4 @@
+  /// 
+  const Registry &registry() const { return _registry; }
+  Registry &registry() { return _registry; }
 
----------------
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 ?

================
Comment at: include/lld/Core/Reference.h:42-43
@@ +41,4 @@
+  enum class KindNamespace { 
+    all     = 0,
+    testing = 1,
+    ELF     = 2,
----------------
What is all, testing ? is this for the core flavor ?

================
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 {
 
----------------
we should delete this file, as its not providing any functionality.

================
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);
----------------
nitpick : adSupport -> addSupport.

================
Comment at: lib/Driver/GnuLdDriver.cpp:339-340
@@ -328,2 +338,4 @@
       }
+      // FIXME: Calling getFileMagic() is is expensive.  It would be better to
+      // wire up the LdScript parser into the registry.
       llvm::sys::fs::file_magic magic = llvm::sys::fs::file_magic::unknown;
----------------
Agree. Perfect place.

================
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) {
----------------
Hexeagon->Hexagon

================
Comment at: lib/ReaderWriter/ELF/DefaultLayout.h:334
@@ -333,3 +333,3 @@
   case DefinedAtom::typeCode:
-    return llvm::StringSwitch<Reference::Kind>(name)
+    return llvm::StringSwitch<Layout::SectionOrder>(name)
       .StartsWith(".eh_frame_hdr", ORDER_EH_FRAMEHDR)
----------------
Thanks for fixing this.


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

BRANCH
  svn

ARCANIST PROJECT
  lld