> On May 27, 2014, at 2:49 PM, Zachary Turner <[email protected]> wrote: > > On Tue, May 27, 2014 at 2:39 PM, <[email protected]> wrote: >> You can't use or make lldb::TargetSP's with the header files that exist on >> the SB side of the LLDB interfaces. They are all shared pointers to opaque >> classes that are only defined in private headers. Nor do any of the SB >> API's return lldb::*SP's, so you couldn't get one that way to pass to a call >> that takes one. It is pretty clear that these are not meant for public >> consumption. >> >> The SetSP/GetSP API's are all private or protected, and the SB classes are >> NOT meant to be derived from by the outside world, so these are explicitly >> not for public consumption. > There are a few cases where they are not private or protected, see the > above list. All of the functions in that list are public (including > at least 3 versions of GetSP / SetSP).
The GetSP/SetSP should probably all be private and friended to whoever needs them, that would be a nice cleanup. But the point is that if you only have the SB API headers, not the _private ones, you can't actually make a lldb::TargetSP. So you can't use an interface that takes one, whether it is private or public. And if you are including the _private headers you are on the inside and we don't guarantee anything about binary compatibility for the lldb_private headers. We might want to document this, or as Greg says include some define to define these out when the SB API's are used naively (though you could always work around this...) > > >> You can't do #2 because all objects are tracked internally using shared >> pointers and sometimes the only thing keeping the backing object alive is >> the shared pointer itself. If you switch to normal pointers the objects >> could disappear and you could end up using the memory pointed to by the raw >> pointer as something that is no longer the type it used to be. > What I meant is that you change this (for example): > > std::shared_ptr<ValueImpl> m_opaque_sp; > > to this: > > std::shared_ptr<ValueImpl>* m_opaque_sp; > > And allocate in the constructor / copy constructor, delete in the > destructor. Then you change every occurrence of m_opaque_sp-> to > (*m_opaque_sp)-> > No, I don't think we should do that either. It just adds ugly boiler plate and the chance for error, and doesn't solve any actual problems. Jim > > On Tue, May 27, 2014 at 2:39 PM, <[email protected]> wrote: >> You can't use or make lldb::TargetSP's with the header files that exist on >> the SB side of the LLDB interfaces. They are all shared pointers to opaque >> classes that are only defined in private headers. Nor do any of the SB >> API's return lldb::*SP's, so you couldn't get one that way to pass to a call >> that takes one. It is pretty clear that these are not meant for public >> consumption. >> >> The SetSP/GetSP API's are all private or protected, and the SB classes are >> NOT meant to be derived from by the outside world, so these are explicitly >> not for public consumption. >> >> As for the others, if you want to put a comment, feel free, though it would >> be kind of annoying to have this boilerplate comment repeated everywhere. >> Might be better to put some policy rules in a common place (maybe LLDB.h or >> SBDefines.h) that says how the SB API's are to be used. >> >> Jim >> >> >> >>> On May 27, 2014, at 2:06 PM, Zachary Turner <[email protected]> wrote: >>> >>> In trying to clean up the warnings in the Windows build, I found the >>> most prominent warning to be C4251 ('identifier' : class 'type' needs >>> to have dll-interface to be used by clients of class 'type2'). This >>> is because almost all of the exported classes (e.g. SBAddress, etc) >>> contain an stl object as part of their interface, usually a smart >>> pointer. This is a bad idea, as described in the following post: >>> http://stackoverflow.com/questions/5661738/how-can-i-use-standard-library-stl-classes-in-my-dll-interface-or-abi >>> >>> The warning can usually be ignored if none of the non-exported classes >>> are made accessible through the public interface of the class, and it >>> seems that at least some effort has been made to see that this is the >>> case, but it does not appear to always be true. I've gone through >>> each of the exported classes, and found the following instances where >>> non-exported classes are exposed through the public interface: >>> >>> SBDebugger(const lldb::DebuggerSP &debugger_sp); >>> SBFrame (const lldb::StackFrameSP &lldb_object_sp); >>> SBProcess (const lldb::ProcessSP &process_sp); >>> SBQueueItem (const lldb::QueueItemSP& queue_item_sp); >>> void SBQueueItem::SetQueueItem (const lldb::QueueItemSP& queue_item_sp); >>> SBTarget (const lldb::TargetSP& target_sp); >>> SBThread (const lldb::ThreadSP& lldb_object_sp); >>> SBValue (const lldb::ValueObjectSP &value_sp); >>> lldb::ValueObjectSP SBValue::GetSP () const; >>> SBWatchpoint (const lldb::WatchpointSP &wp_sp); >>> lldb::WatchpointSP SBWatchpoint::GetSP () const; >>> void SBWatchpoint::SetSP (const lldb::WatchpointSP &sp); >>> >>> I guess there are a couple of approaches to fixing this: >>> >>> 1) Add a comment to each function explaining that it should be used >>> only internally, and suppress the warning. >>> >>> 2) Make the opaque pointer a raw pointer that is manually allocated in >>> the constructor and de-allocated in the destructor. >>> >>> #2 seems like the "best" approach, but it's also more work. Does >>> anyone have strong feelings one way or the other? >>> _______________________________________________ >>> lldb-dev mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev >> _______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
