[lld] registry design proposal

[Michael Spencer]

On Tue, Dec 3, 2013 at 6:18 PM, Nick Kledzik <kledzik at apple.com> wrote:
> There is a design pattern that solves all these problems.  Have a global
> registry of “handlers”.  Require clients to call functions to which register
> the handlers they request.  Then any code that needs a handler calls the
> global registry to call the needed handler.
>
> I see the need for three registries:  a file parser registry, a yaml
> document tag registry, and a kind string registry.

What would the kind string registry look like? Is the intent for
converting from Native -> YAML -> Native? I assume the namespace would
just be {COFF, ELF, MachO}, <arch>.

>
> Here is one way to implement a registry for the file parser:
>
> class InputParserRegistry {
> public:
>   // A function to quickly check if a file can be parsed.  Usually only the
>   // magic parameter is checked.  But it can also check the file extension,
> or
>   // even look at more of the buffer if needed.
>   typedef bool (*InputFileChecker)(file_magic magic, StringRef
> fileExtension,
>                                    const MemoryBuffer &mb);
>
>   // A function to parse the supplied input file.
>   typedef error_code (*InputFileParser)(std::unique_ptr<MemoryBuffer> &mb,
>                                 std::vector<std::unique_ptr<File>> &result);
>
>   // Registers a checker and parser with the global registry.
>   // Usually, the checker and parser are static methods in some lld::File
>   // subclass that will be instantiated.
>   static void registerParser(InputFileChecker checker,
>                              InputFileParser parser);

I would rather use std::function over raw function pointers.

>
>   // Walks registered checkers to parse the specified file.
>   static error_code parseFile(std::unique_ptr<MemoryBuffer> &mb,
>                               std::vector<std::unique_ptr<File>> &result);
> };
>
>
> The mach-o linker tool would call:
>    registryAddSupportMachO();
>    registryAddSupportArchives();
>    registryAddSupportNative();
>
> The implementation of registryAddSupportMachO would look like:
>   void registryAddSupportMachO() {
>     InputParserRegistry::registerParser(FileMachO::check, FileMachO::parse);
>   }
>
> The code (in custom InputGraph nodes) no longer needs to call
> identify_magic, or look at the file extension, or handle archives.  All it
> does is:
>   InputParserRegistry::parseFile(_buffer, _files);
>
> I’ve looked at the llvm::Registry<> classes.  It does not seem to be a good
> match for what we need here:  It is designed around using static variables
> whose construction does the registration.  I prefer explicit calls to
> register to enable clients to *not* drag in support they don’t want.  The
> Registry<> model also assume you want to instantiate the object with the
> default constructor, which we don’t.  And we have no need for listeners or
> name/description. Yes, by overriding SimpleRegistryEntry and RegistryTraits
> we could use  llvm::Registry<>, but I’m not sure it is worth the effort.

So the major difference would be a pull versus push model? I think
given the use case for lld that the proposed pull model is fine.

>
>
> Overall, I want to see what you think of this pattern.  Once we have it
> worked out, I want to do a similar registry for:
> 1) yaml doc tags (so that various kinds of yaml (archive, atom, mach-o, elf,
> etc) can be intermixed in one test case.
> 2) Add a namespace registry for kind values.
>
> -Nick
>

The general idea sounds good to me. I think it would also solve part
of the circular dependency problem.

- Michael Spencer