[lldb-dev] Object identities in the LLDB's C++ API

Tue Jan 29 10:47:28 PST 2019

Reviving this old thread and +Joshua Peraza <jperaza at google.com>  who's
also interested in this.

On Wed, Dec 13, 2017 at 4:17 PM Leonard Mosescu <mosescu at google.com> wrote:

> Thanks Greg,
>
> 1. Expose the opaque ptr as an opaque handle()
>      - this is an easy, quick and convenient solution for many SBxxx types
> but it may not work for all
>
> That would be nice, but that won't always work with how LLDB is currently
>> coded for SBFrame and possibly SBThread. These objects will be problems as
>> they can come and go and the underlying object isn't always the same even
>> through they lock onto the same logical object. SBThread and SBFrame have
>> "lldb::ExecutionContextRefSP m_opaque_sp" members. The execution context
>> reference is a class that contains weak pointers to the
>> lldb_private::Thread and lldb_private::StackFrame objects, but it also
>> contains the thread ID and frame ID so it can reconstitute the
>> value lldb_private::Thread and lldb_private::StackFrame even if the weak
>> pointer isn't valid. So the opaque handle will work for many objects but
>> not all.
>
>
> Indeed. One, relatively small but interesting benefit of the opaque handle
> type is that it opens the possibility of generic "handle maps" (I'll
> elaborate below)
>
> 2. Design and implement a consistent, first class
> identity/ordering/hashing for all the SBxxx types
>      - perhaps the most elegant and flexible approach, but also the most
> work
>
> I would be fine with adding new members to classes we know we want to hash
>> and order, like by adding:
>> uint32_t SB*::GetHash();
>> bool SB*::operator==(const SB*& ohs);
>> bool SB*::operator<(const SB*& ohs);
>> Would those be enough?
>
>
> I think so. If we use the standard containers as reference, technically we
> only need operator< to satisfy the Compare
> <http://en.cppreference.com/w/cpp/concept/Compare> concept. (also, a
> small nit - size_t would be a better type for the hash value). Also, both
> the hashing and the compare can be implemented as non-member functions (or
> even specializing std::hash, std::less for SBxxx types). A few minor
> concerns:
>
> a. if we keep things like SBModule::operator==() unchanged, it's not going
> to be the same as the equiv(a, b) for the case where a and b have null
> opaque pointers (not sure if this breaks anything, but I wouldn't want to
> be the first to debug a case where this matter)
> b. defining just the minimum set of operations may be technically enough
> but it may look a bit weird to have a type define < but none of the other
> relational operators.
> c. if some of the hash/compare implementation end up going through
> multiple layers (the execution context with thread, frame IDs example) the
> performance characteristics can be unpredictable, right?
>
>
> For context, the use case that brought this to my attention is managing a
> set of data structures that contain custom data associated with modules,
> frames, etc. It's easy to create, let's say a MyModule from a SBModule, but
> if later on I get the module for a particular frame, SBFrame::GetModule()
> will return a SBModule, which I would like to map to the corresponding
> MyModule instance. Logically this would require a SBModule -> MyModule map.
> The standard associative containers (map or unordered_map) would make this
> trivial if SBxxx types satisfy the key requirements.
>
> Another option for maintaining such a mapping, suggested by Mark Mentovai,
> is to use provision for an "user data" tag associated with every SBxxx
> object (this tag can simply be a void*, maybe wrapped with type safe
> accessors). This would be extremely convenient for the API users (since
> they don't have to worry about maintaining any maps themselves) but
> implementing it would hit the same complications around the synthesized
> instances (like SBFrame) and it may carry a small price - one pointer per
> SBxxx instance even if this facility is not used. I personally like this
> approach and in this particular case it has the additional benefit of being
> additive (we can graft it on with minimal risk of breaking existing stuff),
> although it still seems nice to have consistent identity semantics for the
> SBxxx types.
>
> On Wed, Dec 13, 2017 at 12:40 PM, Greg Clayton <clayborg at gmail.com> wrote:
>
>>
>> On Dec 13, 2017, at 11:44 AM, Leonard Mosescu via lldb-dev <
>> lldb-dev at lists.llvm.org> wrote:
>>
>> LLDB's C++ API deals with SBxxx objects, most of which are PIMPL-style
>> wrappers around an opaque pointer to the internal implementation. These
>> SBxxx objects act as handles and are passed/returned by value, which is
>> generally convenient, except for the situations where one would need to
>> keep track of object identities, ex. using them as keys in associative
>> containers.
>>
>> As far as I can tell, there's a bit of inconsistency in the current state:
>>
>> 1. Some types, ex. SBThread, SBTarget, SBSection, ... offer ==, != that
>> map directly to the corresponding operator on the opaque pointer (good!),
>> but:
>>     .. there are no ordering operators, nor obvious ways to hash the
>> objects
>> 2. SBModule offer the == , != operators, but:
>>     ... the implementations for == and != are not exactly forwarded to
>> the corresponding operator on the opaque pointer (1)
>> 3. Things like SBFrame offer IsEqual() in addition to ==, !=, creating a
>> bit of confusion
>> 4. Other types (ex. SBProcess, SBSymbol, SBBlock) don't offer any kind of
>> comparison operations.
>>
>> IMO it would be nice to have a consistent "handle type" semantics
>> regarding identity, ordering and hashing. I can see the following options:
>>
>> 1. Expose the opaque ptr as an opaque handle()
>>      - this is an easy, quick and convenient solution for many SBxxx
>> types but it may not work for all
>>
>>
>> That would be nice, but that won't always work with how LLDB is currently
>> coded for SBFrame and possibly SBThread. These objects will be problems as
>> they can come and go and the underlying object isn't always the same even
>> through they lock onto the same logical object. SBThread and SBFrame have
>> "lldb::ExecutionContextRefSP m_opaque_sp" members. The execution context
>> reference is a class that contains weak pointers to the
>> lldb_private::Thread and lldb_private::StackFrame objects, but it also
>> contains the thread ID and frame ID so it can reconstitute the
>> value lldb_private::Thread and lldb_private::StackFrame even if the weak
>> pointer isn't valid. So the opaque handle will work for many objects but
>> not all.
>>
>>
>> 2. Design and implement a consistent, first class
>> identity/ordering/hashing for all the SBxxx types
>>      - perhaps the most elegant and flexible approach, but also the most
>> work
>>
>>
>> I would be fine with adding new members to classes we know we want to
>> hash and order, like by adding:
>>
>> uint32_t SB*::GetHash();
>> bool SB*::operator==(const SB*& ohs);
>> bool SB*::operator<(const SB*& ohs);
>>
>> Would those be enough?
>>
>>
>> Any thoughts on this? Did I miss anything fundamental here?
>>
>>
>>
>> Thanks,
>> Lemo.
>>
>> (1) example of operator== from SBModule:
>>
>> bool SBModule::operator==(const SBModule &rhs) const {
>> if (m_opaque_sp)
>> return m_opaque_sp.get() == rhs.m_opaque_sp.get();
>> return false;
>> }
>>
>>
>> So I would leave this up to the SB classes so that we can change the
>> implementation if needed so I would prefer to add methods to each SB object
>> for hashing and comparison.
>>
>> Greg Clayton
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20190129/8b6d04f8/attachment.html>