<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">It would be easy to add the equality or comparison and other operators to any items that don't have them. I am not a fan of adding user data. It can't live be in the SBXXX object itself, because we can't change the member variables in the objects for API backward compat issues. If it is inside the object itself, and we have only 1 user data "void *" per object, I can see many clients wanting to use this user data. I would rather people construct their own maps that don't live in the object, but that gets back to adding hashing to the items. So I would vote to allow the items to be hashed where needed. I am not sure we need hashing for SBFrame or SBThread since they have really well defined IDs in their APIs. SBFrame has the index ID and CFA (call frame address or SP at start of the function). SBThread has the <span style="background-color: rgb(255, 255, 255);" class="">index ID (a unique number that starts at 1 and </span>increments as new threads come along). But if you feel we need them it is fine to add them to any objects that need it. I always use the file path for the SBModule as the key for any maps when I use SBModule objects in a map. Let me know what you think after these comments<div class=""><div class=""><br class=""></div><div class=""><br class=""><div class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Jan 29, 2019, at 10:47 AM, Leonard Mosescu <<a href="mailto:mosescu@google.com" class="">mosescu@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Reviving this old thread and <a class="gmail_plusreply" id="plusReplyChip-0" href="mailto:jperaza@google.com" tabindex="-1">+Joshua Peraza</a> who's also interested in this.<br class=""></div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Dec 13, 2017 at 4:17 PM Leonard Mosescu <<a href="mailto:mosescu@google.com" class="">mosescu@google.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class=""><div class="">Thanks Greg,<br class=""><blockquote type="cite" style="color:rgb(80,0,80);font-size:12.8px" class=""><div dir="ltr" class=""><div class="">1. Expose the opaque ptr as an opaque handle() </div><div class=""> - this is an easy, quick and convenient solution for many SBxxx types but it may not work for all</div></div></blockquote></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px" class="">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 </span><span style="font-size:12.8px" class="">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.</span></blockquote><div class=""><br class=""></div><div class="">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)</div><div class=""><blockquote type="cite" style="color:rgb(80,0,80);font-size:12.8px" class=""><div dir="ltr" class=""><div class="">2. Design and implement a consistent, first class identity/ordering/hashing for all the SBxxx types</div><div class=""> - perhaps the most elegant and flexible approach, but also the most work</div></div></blockquote></div><div class=""><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" class="gmail_quote">I would be fine with adding new members to classes we know we want to hash and order, like by adding:<br class="">uint32_t SB*::GetHash();<br class="">bool SB*::operator==(const SB*& ohs);<br class="">bool SB*::operator<(const SB*& ohs);<br class="">Would those be enough?</blockquote></div><div class=""> </div><div class="">I think so. If we use the standard containers as reference, technically we only need operator< to satisfy the <a href="http://en.cppreference.com/w/cpp/concept/Compare" target="_blank" class="">Compare</a> 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:</div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px" class=""><div class="">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)</div><div class="">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.</div><div class="">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? </div></blockquote><div class=""><br class=""></div><div class="">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. </div><div class=""><br class=""></div><div class="">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. </div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Wed, Dec 13, 2017 at 12:40 PM, Greg Clayton <span dir="ltr" class=""><<a href="mailto:clayborg@gmail.com" target="_blank" class="">clayborg@gmail.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="overflow-wrap: break-word;" class=""><br class=""><div class=""><span class=""><blockquote type="cite" class=""><div class="">On Dec 13, 2017, at 11:44 AM, Leonard Mosescu via lldb-dev <<a href="mailto:lldb-dev@lists.llvm.org" target="_blank" class="">lldb-dev@lists.llvm.org</a>> wrote:</div><br class="gmail-m_5038734868273431791m_4182136635983653466Apple-interchange-newline"><div class=""><div dir="ltr" class="">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.<div class=""><br class=""></div><div class="">As far as I can tell, there's a bit of inconsistency in the current state:</div><div class=""><br class=""></div><div class="">1. Some types, ex. SBThread, SBTarget, SBSection, ... offer ==, != that map directly to the corresponding operator on the opaque pointer (good!), but:</div><div class=""> .. there are no ordering operators, nor obvious ways to hash the objects</div><div class="">2. SBModule offer the == , != operators, but:</div><div class=""> ... the implementations for == and != are not exactly forwarded to the corresponding operator on the opaque pointer (1)<br class=""></div><div class="">3. Things like SBFrame offer IsEqual() in addition to ==, !=, creating a bit of confusion</div><div class="">4. Other types (ex. SBProcess, SBSymbol, SBBlock) don't offer any kind of comparison operations.</div><div class=""><br class=""></div><div class="">IMO it would be nice to have a consistent "handle type" semantics regarding identity, ordering and hashing. I can see the following options:</div><div class=""><br class=""></div><div class="">1. Expose the opaque ptr as an opaque handle() </div><div class=""> - this is an easy, quick and convenient solution for many SBxxx types but it may not work for all</div></div></div></blockquote><div class=""><br class=""></div></span>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.<span class=""><br class=""><blockquote type="cite" class=""><div dir="ltr" class=""></div></blockquote><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="">2. Design and implement a consistent, first class identity/ordering/hashing for all the SBxxx types</div><div class=""> - perhaps the most elegant and flexible approach, but also the most work</div></div></div></blockquote><div class=""><br class=""></div></span>I would be fine with adding new members to classes we know we want to hash and order, like by adding:</div><div class=""><br class=""></div><div class="">uint32_t SB*::GetHash();</div><div class="">bool SB*::operator==(const SB*& ohs);</div><div class=""><div class="">bool SB*::operator<(const SB*& ohs);</div><div class=""><br class=""></div><div class="">Would those be enough?</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""></div></blockquote></div></div><span class=""><div class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">Any thoughts on this? Did I miss anything fundamental here?</div></div></div></blockquote><div class=""><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Lemo.</div><div class=""><br class=""></div><div class="">(1) example of operator== from SBModule:</div><div class=""><br class=""></div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px" class=""><div class=""><div style="font-family:"Droid Sans Mono",monospace,monospace,"Droid Sans Fallback";line-height:18px;white-space:pre-wrap" class=""><div class=""><font size="1" class=""><span style="color:rgb(0,0,255)" class="">bool</span> SBModule::<span style="color:rgb(175,0,219)" class="">operator</span>==(<span style="color:rgb(0,0,255)" class="">const</span> SBModule &rhs) <span style="color:rgb(0,0,255)" class="">const</span> {</font></div></div></div><div class=""><div style="font-family:"Droid Sans Mono",monospace,monospace,"Droid Sans Fallback";line-height:18px;white-space:pre-wrap" class=""><div class=""><font size="1" class=""> <span style="color:rgb(175,0,219)" class="">if</span> (m_opaque_sp)</font></div></div></div><div class=""><div style="font-family:"Droid Sans Mono",monospace,monospace,"Droid Sans Fallback";line-height:18px;white-space:pre-wrap" class=""><div class=""><font size="1" class=""> <span style="color:rgb(175,0,219)" class="">return</span> m_opaque_sp.<span style="color:rgb(121,94,38)" class="">get</span>() == rhs.<span style="color:rgb(0,16,128)" class="">m_opaque_sp</span>.<span style="color:rgb(121,94,38)" class="">get</span>();</font></div></div></div><div class=""><div style="font-family:"Droid Sans Mono",monospace,monospace,"Droid Sans Fallback";line-height:18px;white-space:pre-wrap" class=""><div class=""><font size="1" class=""> <span style="color:rgb(175,0,219)" class="">return</span> <span style="color:rgb(0,0,255)" class="">false</span>;</font></div></div></div><div class=""><div style="font-family:"Droid Sans Mono",monospace,monospace,"Droid Sans Fallback";line-height:18px;white-space:pre-wrap" class=""><div class=""><font size="1" class="">}</font></div></div></div></blockquote></div></div></blockquote><br class=""></div></span><div class="">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.</div><span class="gmail-m_5038734868273431791HOEnZb"><font color="#888888" class=""><div class=""><br class=""></div><div class="">Greg Clayton</div><div class=""><br class=""></div><br class=""></font></span></div></blockquote></div><br class=""></div>
</blockquote></div>
</div></blockquote></div><br class=""></div></div></div></body></html>