- 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)
- 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:
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 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.