[PATCH] D64968: [Object] Create MutableObject class for doing Object Mutations [Part 2]

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 19 04:02:20 PDT 2019


jhenderson added a comment.

It looks like in this patch, you haven't uploaded the diff versus D64281 <https://reviews.llvm.org/D64281>, it's versus trunk. As previously explained, I think it's important you do the diff versus the previous patch, so that it's obvious what this adds over the previous one.

I also don't think this is the right next step, or at least it's too much for one patch. I think a better thing to do would be to add only the bare minimum number of sections you need to be able to remove a section from a very basic ELF (e.g. one with just a .data section in with no symbols). In this case, you probably just need a string table section, so that you can update it's contents following the removal of sections. That would allow you to write unit tests that test some useful functionality. Subsequent patches can add proper handling of other section types, so that the ELF is not invalidated following removal of certain sections, but in the first instance, you don't need to handle everything.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64968/new/

https://reviews.llvm.org/D64968





More information about the llvm-commits mailing list