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 <jing...@apple.com> wrote: > > > On Jan 31, 2017, at 3:59 PM, Zachary Turner via Phabricator via > lldb-commits <lldb-commits@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@lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
