[Lldb-commits] [PATCH] D30560: Split DataExtractor into two classes.

[Jim Ingham via lldb-commits]

That sounds right.  Please do fix the function names.  We can have another discussion about naming at some point, but we shouldn't do it piecemeal.

Jim

> On Mar 3, 2017, at 9:53 AM, Zachary Turner <zturner at google.com> wrote:
> 
> Yea, I can see splitting the target specific stuff into a separate "target aware" dump function and having the base one not require the exeuction context and go in Utility.  In the interest of not breaking anything it seems reasonable to try to do that as a separate patch so we the functional and non functional change portions can be separated from each other.
> 
> On Fri, Mar 3, 2017 at 9:51 AM Jim Ingham via Phabricator <reviews at reviews.llvm.org> wrote:
> jingham added a comment.
> 
> Yes, that is a good solution.  It's still a little awkward that DataExtractors live in Utility and their dumper lives in Core.  Especially as almost all the functionality of the dumper could live in Utility.  If you were serious about using Utility separate from Core you would want to put the non-target parts of DumpDataExtractor into Core and assert if those are passed the unsupported flavors, and then have DumpDataExtractorTarget, with the instruction and fancy float bits.  But maybe that's work for another day.
> 
> We name functions starting with a Capitol first letter, so these should be "Dump..." not "dump...".  But other than that, this seems good.
> 
> 
> https://reviews.llvm.org/D30560
> 
> 
>