[PATCH] D53945: [TextAPI] TBD Reader/Writer

Armando Montanez via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 6 14:41:19 PST 2018


amontanez added a comment.

In https://reviews.llvm.org/D53945#1289303, @jakehehrlich wrote:

> The interface file has *a lot* of MachO specific things in it. It also isn't clear that even Architecture may not even be easily dedupable IMO. The registry interface seems more on the level of split I was expecting. It demands a raw_ostream for the output which isn't ideal IMO because that's not really how you'd ever want to output an ELF file. I focused on that in my review because of that.


Are you suggesting the Registry has a notion of two (or more) different file definitions (i.e. MachOInterfaceFile and ELFInterfaceFile), or am I misinterpreting?

To clarify, I don't think the API of InterfaceFile could be shared, I was intending to suggest something similar to `llvm::object::ObjectFile` that would be used in the contexts that InterfaceFile is currently used in. When more specifics are needed (for example, in TextStub.cpp), if the FileType is appropriate then you can downcast to the class with the additional functionality. Does keeping a general interface in that way reduce code duplication enough to be justified? I think it depends on what the tool ends up doing.


https://reviews.llvm.org/D53945





More information about the llvm-commits mailing list