[Lldb-commits] [PATCH] D29359: Start breaking some dependencies in lldbUtility

Tue Jan 31 16:37:31 PST 2017

Thanks for the clarification, I wrote "try to convert" there because I
didn't look closely enough to see what problem it was solving.  Since it
seems to be specific to ValueObject (which a grep confirms), we may be able
to just move the code for SharingPtr next to ValueObject.  That said,
SharingPtr.h / .cpp don't actually have any reverse dependencies, so we
could also just leave it alone.  I mostly just put that on the list because
if we intend for this to be a general purpose library of reusable stuff, we
might as well remove things that are not seeing broad use.

On Tue, Jan 31, 2017 at 4:32 PM Jim Ingham <jingham at apple.com> wrote:

>
> > On Jan 31, 2017, at 3:59 PM, Zachary Turner via Phabricator via
> lldb-commits <lldb-commits at lists.llvm.org> wrote:
> >
> > zturner created this revision.
> > Herald added a subscriber: mgorny.
> >
> > The goal here is to make `lldbUtility` a library which depends on no
> other libraries.  It seems like this library already exists in spirit as a
> place to house very low level code which is commonly used by all parts of
> LLDB, so it makes sense to designate this as the one top-level library.  We
> can change the name in the future to something like `Support` if we choose
> to (implying that it is analogous to LLVMSupport), but for now I just want
> to break the dependencies.
> >
> > So, this patch deletes a bunch of dead code and moves some code that is
> actually only used in 1-2 places up to where it's actually used.
> >
> > This is not enough to get `Utility` dependency free.  Further tasks that
> I've identified, but which are too large to fit into this patch, include:
> >
> > 1. Move `RegularExpression` from Core -> Utility  (Long term: Delete and
> use LLVM)
> > 2. Move `Stream` from Core -> Utility  (Long term: Delete and use
> llvm::raw_ostream)
> > 3. Move `ConstString` from Core -> Utility
> > 4. Move `ProcessStructReader` from Utility -> Process
> > 5. Move `RegisterNumber` from Utility -> Target
> > 6. Move `Error` from Core -> Utility
> > 7. Try to convert `ValueObject::GetSP()` from SharingPtr to
> `std::shared_ptr`, then delete `SharingPtr`.
>
> The problem SharingPtr is solving is that the value objects form a
> cluster: the parent value object (representing some structure say) and any
> of it's children (the subelement or the subelement of the subelement...)
> that have been realized by the ChildAtIndex, ChildByPath, and Co. calls.
> And the way the value objects work, the whole cluster has to stay alive as
> long as any of the children are alive.  That's because everybody just knows
> where they are in their parent (as they should since they all can move
> around.)  So you need them all for jobs like recomputing values when the
> stop ID changes.   But users are not (and shouldn't be) required to
> manually hold onto any more than the element they care about, which may
> very well be a deep child.  SharingPtr manages this cluster and makes sure
> it stays alive as long as any members of the cluster are alive.
>
> I have no particular stake in how this is done, and if there's something
> clever in llvm that can do the same job, I'm all for that.  But you can't
> replace it with a std::shared_ptr, that won't work.
>
> Jim
>
>
> > 8. Move `ModuleCache` from Utility -> Target
> >
> > Finally, once all of those things are done, we can begin breaking up the
> `lldb-private.h`, and `lldb-enumerations.h`, etc header files and moving
> the enumerations to more appropriate locations, which will finally break
> this up.
> >
> >
> > https://reviews.llvm.org/D29359
> >
> > Files:
> >  lldb/include/lldb/Utility/ConvertEnum.h
> >  lldb/include/lldb/Utility/PriorityPointerPair.h
> >  lldb/include/lldb/Utility/Utils.h
> >  lldb/include/lldb/lldb-private-enumerations.h
> >  lldb/source/Commands/CommandObjectPlatform.cpp
> >  lldb/source/Core/Section.cpp
> >  lldb/source/Interpreter/OptionGroupArchitecture.cpp
> >  lldb/source/Interpreter/OptionGroupFormat.cpp
> >  lldb/source/Interpreter/OptionGroupOutputFile.cpp
> >  lldb/source/Interpreter/OptionGroupPlatform.cpp
> >  lldb/source/Interpreter/OptionGroupUUID.cpp
> >  lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp
> >  lldb/source/Interpreter/OptionGroupVariable.cpp
> >  lldb/source/Interpreter/OptionGroupWatchpoint.cpp
> >  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
> >  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
> >  lldb/source/Target/Platform.cpp
> >  lldb/source/Target/ThreadList.cpp
> >  lldb/source/Target/ThreadPlan.cpp
> >  lldb/source/Utility/ARM64_DWARF_Registers.cpp
> >  lldb/source/Utility/ARM64_DWARF_Registers.h
> >  lldb/source/Utility/ARM_DWARF_Registers.cpp
> >  lldb/source/Utility/ARM_DWARF_Registers.h
> >  lldb/source/Utility/CMakeLists.txt
> >  lldb/source/Utility/ConvertEnum.cpp
> >
> > <D29359.86516.patch>_______________________________________________
> > lldb-commits mailing list
> > lldb-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170201/f99bb413/attachment-0001.html>