[Lldb-commits] [PATCH] Expose ThreadCollection in SB API

[Enrico Granata]

Couple things:

- why do you need stdio.h in your header file?
+#include <stdio.h>

- is there any reason why this constructor is public?
+    SBThreadCollection (const lldb::ThreadCollectionSP &thread_list);

We are usually trying to keep the constructors that take SPs protected so they are not exposed to API clients; unless you have a compelling reason, I’d do the same here

For instance, SBData has the following:
protected:
    
    // Mimic shared pointer...
    lldb_private::DataExtractor *
    get() const;
    
    lldb_private::DataExtractor *
    operator->() const;
    
    lldb::DataExtractorSP &
    operator*();
    
    const lldb::DataExtractorSP &
    operator*() const;

    SBData (const lldb::DataExtractorSP &data_sp);

    void
    SetOpaque (const lldb::DataExtractorSP &data_sp);

You could so something similar in your SBThreadCollection; provide protected accessors, constructor and setter, and only keep the default & copy constructors public

On a more philosophical note - is SBThreadCollection meant as a “read-only” snapshot of a set of threads as they existed at one time?
If so, then it’s all fine and dandy
But it seems that this class lacks any write access; I can’t create a new thread collection and start adding threads to it
Is this intended in your design, or did you just not feel the need to allow that?
Mostly curious..

> On Sep 5, 2014, at 2:20 PM, Kuba Brecka <kuba.brecka at gmail.com> wrote:
> 
> Another piece of the puzzle, let's expose the newly abstracted ThreadCollection into SB API, based on http://reviews.llvm.org/D5200 and another patch that will actually provide an SBThreadCollection of ASan history threads is coming next.
> 
> http://reviews.llvm.org/D5218
> 
> Files:
>  include/lldb/API/SBDefines.h
>  include/lldb/API/SBThreadCollection.h
>  lldb.xcodeproj/project.pbxproj
>  scripts/Python/build-swig-Python.sh
>  scripts/Python/buildSwigPython.py
>  scripts/Python/interface/SBThreadCollection.i
>  scripts/lldb.swig
>  source/API/SBThreadCollection.cpp
> <D5218.13339.patch>_______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Thanks,
- Enrico
📩 egranata@.com ☎️ 27683




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140905/af6e1d97/attachment.html>