[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