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

Wed Dec 13 16:17:18 PST 2017

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/20171213/07ee6d10/attachment-0001.html>