This revision was automatically updated to reflect the committed changes.
Closed by commit rL296910: Isolate Target-specific functionality of
DataExtractor. (authored by zturner).
Changed prior to commit:
https://reviews.llvm.org/D30560?vs=90432&id=90526#toc
Repository:
rL LLVM
https://revi
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 wrote:
>
> Yea, I can see splitting the target specific stuff into a separate "target
> aware" d
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 n
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 yo
labath added a comment.
I like the free-standing function approach a lot.
https://reviews.llvm.org/D30560
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
zturner updated this revision to Diff 90432.
zturner added a comment.
I basically turned the two dump methods into free functions and moved the DWARF
specific function into `DWARFCallFrameInfo.cpp`. I think this is much better
than before, as we don't muck with the inheritance hierarchy at all
I was actually thinking of making the dump functions free functions that
take a const DataExtractor&. This way the entire implementation could
remain unchanged with the exception of replacing implicit member variable
reads with reads through an explicit instance of the extractor.
This way we don't
jingham added a comment.
The EH stuff should definitely go in a specific subclass.
It also seems a shame to lose the capability to Dump DataExtractors in general.
The only thing the exe_scope is used for in Dump (which is what is causing you
problems) is to print instructions, and to do a more
Tbh I felt dirty calling it Ex, so thanks for calling me out on it :)
I'll whip up some changes later
On Thu, Mar 2, 2017 at 6:09 PM Jim Ingham via Phabricator <
revi...@reviews.llvm.org> wrote:
> jingham added a comment.
>
> Is it horrible of me to ask that we choose a name that is more descript
jingham added a comment.
Is it horrible of me to ask that we choose a name that is more descriptive than
DataExtractorEx? That "Ex" extension to a class name is just such a punt, and
I'd really rather not encourage it.
The subclass you made adds the ability to:
(a) Dump itself
(b) Extract som
10 matches
Mail list logo
