This revision was automatically updated to reflect the committed changes.
Closed by commit rL337832: Move dumping code out of RegisterValue class
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D48351
Files:
lldb/tru
labath added a comment.
In https://reviews.llvm.org/D48351#1169959, @jingham wrote:
> Dump is really meant to be the private description of the object that you
> would use for logging and the like - Description was the public face of a
> class. So while the Description-like functionality could
jingham added a comment.
Dump is really meant to be the private description of the object that you would
use for logging and the like - Description was the public face of a class. So
while the Description-like functionality could have a restrictions that
constrain its use to pretty high up in
This is the current list of files (after this patch) that call
DumpDataExtractor or DumpRegisterValue
source/API/SBData.cpp
source/Commands/CommandObjectMemory.cpp
source/Commands/CommandObjectRegister.cpp
source/Core/Address.cpp
source/Core/EmulateInstruction.cpp
source/Core/Event.cpp
source/Core
Well, if it depends on everything, then it can only used from places
that also depend on everything. Right now I don't think it matters now
(though it will create a new Dump <-> "everything calling Dump"
cycle). However, I can imagine this might cause some tricky situations
later on when we try to
We only use dumping from lldb-test, from inside the debugger, or from the
top level command interpreter. All of those things necessarily depend on
everything anyway.
On Fri, Jul 20, 2018 at 8:02 AM Pavel Labath wrote:
> Well, if it depends on everything, then it can only used from places
> that a
labath added subscribers: clayborg, jingham, zturner.
labath added a comment.
Well, if it depends on everything, then it can only used from places
that also depend on everything. Right now I don't think it matters now
(though it will create a new Dump <-> "everything calling Dump"
cycle). However,
zturner added a subscriber: labath.
zturner added a comment.
I had previously thought of making a top level project called Dump that
depends on everything. Also makes it very obvious where all the dumpers
are. It can have overloaded functions called lldb_private::dump(T&) for
every value of T, the
I had previously thought of making a top level project called Dump that
depends on everything. Also makes it very obvious where all the dumpers
are. It can have overloaded functions called lldb_private::dump(T&) for
every value of T, then no matter what type you have, all you have to do is
call dum
labath added a comment.
In https://reviews.llvm.org/D48351#1168384, @jingham wrote:
> If this pattern becomes more common, then we have to deal with how somebody
> would find these dump routines. If RegisterValue gets moved out of Core,
> would you really look to a file in Core for the dump ro
clayborg added a comment.
I am fine if this helps with not pulling in the world. Seems like it should
move out of Core then no? Seems weird to make the change yet keep it in the
same directory.
https://reviews.llvm.org/D48351
___
lldb-commits mail
labath added a comment.
The Stream part isn't the problem. The problem is that the dumping code is
implemented in terms of `ExecutionContextScope`, which then pulls in pretty
much everything. If it was just the object "dumping itself" then I would be
fine with it as a method, but here it's usin
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.
I don't agree that moving dump routines away from the class that they are
dumping is a good idea in general. It makes it harder for somebody new to the
code to find out how to print out the
clayborg added a comment.
Is this a case of keeping RegisterValue from needing to link against the stream
stuff so code inside a directory doesn't depend on code from other directories?
Not sure why we wouldn't want to just ask the object itself to dump itself...
https://reviews.llvm.org/D4835
zturner added a comment.
Seems good to me. Separating dump code from core logic is always helpful.
https://reviews.llvm.org/D48351
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide added a comment.
I guess this is fine. @jingham?
https://reviews.llvm.org/D48351
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
labath added a comment.
Does anyone have an opinion on this?
https://reviews.llvm.org/D48351
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
labath created this revision.
labath added reviewers: zturner, jingham, clayborg.
Herald added a subscriber: mgorny.
The dump function was the only part of this class which depended on
high-level functionality. This was due to the DumpDataExtractor
function, which uses info from a running target t
18 matches
Mail list logo
