<div dir="ltr">Will update the patch.<div><br></div><div>I don't have a strong opinion whether SBThreadCollection should or shouldn't be read-only. It seems to me its purpose of existence now is to allow SB API to return multiple SBThreads as a single return value. Can't think of any API that would be suitable to have a SBThreadCollection as an input (that the user would construct and populate). If you think we should allow it, I can add it.</div><div><br></div><div>Kuba<div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 5, 2014 at 2:35 PM, Enrico Granata <span dir="ltr"><<a href="mailto:egranata@apple.com" target="_blank">egranata@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word">Couple things:<div><br></div><div>- why do you need stdio.h in your header file?</div><div><div>+#include <stdio.h></div></div><div><br></div><div>- is there any reason why this constructor is public?</div><div><div>+    SBThreadCollection (const lldb::ThreadCollectionSP &thread_list);</div></div><div><br></div><div>We are usually trying to keep the constructors that take SP<i>s</i> protected so they are not exposed to API clients; unless you have a compelling reason, I’d do the same here</div><div><br></div><div>For instance, SBData has the following:</div><div><div style="margin:0px;font-size:11px;font-family:Menlo;color:rgb(187,44,162)">protected<span style="color:rgb(0,0,0)">:</span></div><p style="margin:0px;font-size:11px;font-family:Menlo;min-height:13px">    <br></p><div style="margin:0px;font-size:11px;font-family:Menlo;color:rgb(0,132,0)"><span style="color:rgb(0,0,0)">    </span>// Mimic shared pointer...</div><div style="margin:0px;font-size:11px;font-family:Menlo;color:rgb(79,129,135)"><span style="color:rgb(0,0,0)">    </span>lldb_private<span style="color:rgb(0,0,0)">::</span>DataExtractor<span style="color:rgb(0,0,0)"> *</span></div><div style="margin:0px;font-size:11px;font-family:Menlo">    get() <span style="color:rgb(187,44,162)">const</span>;</div><p style="margin:0px;font-size:11px;font-family:Menlo;min-height:13px">    <br></p><div style="margin:0px;font-size:11px;font-family:Menlo;color:rgb(79,129,135)"><span style="color:rgb(0,0,0)">    </span>lldb_private<span style="color:rgb(0,0,0)">::</span>DataExtractor<span style="color:rgb(0,0,0)"> *</span></div><div style="margin:0px;font-size:11px;font-family:Menlo;color:rgb(187,44,162)"><span style="color:rgb(0,0,0)">    </span>operator<span style="color:rgb(0,0,0)">->() </span>const<span style="color:rgb(0,0,0)">;</span></div><p style="margin:0px;font-size:11px;font-family:Menlo;min-height:13px">    <br></p><div style="margin:0px;font-size:11px;font-family:Menlo;color:rgb(79,129,135)"><span style="color:rgb(0,0,0)">    </span>lldb<span style="color:rgb(0,0,0)">::</span>DataExtractorSP<span style="color:rgb(0,0,0)"> &</span></div><div style="margin:0px;font-size:11px;font-family:Menlo">    <span style="color:rgb(187,44,162)">operator</span>*();</div><p style="margin:0px;font-size:11px;font-family:Menlo;min-height:13px">    <br></p><div style="margin:0px;font-size:11px;font-family:Menlo;color:rgb(79,129,135)"><span style="color:rgb(0,0,0)">    </span><span style="color:rgb(187,44,162)">const</span><span style="color:rgb(0,0,0)"> </span>lldb<span style="color:rgb(0,0,0)">::</span>DataExtractorSP<span style="color:rgb(0,0,0)"> &</span></div><div style="margin:0px;font-size:11px;font-family:Menlo;color:rgb(187,44,162)"><span style="color:rgb(0,0,0)">    </span>operator<span style="color:rgb(0,0,0)">*() </span>const<span style="color:rgb(0,0,0)">;</span></div><div style="margin:0px;font-size:11px;font-family:Menlo;min-height:13px"><br></div><div style="margin:0px;font-size:11px;font-family:Menlo">    SBData (<span style="color:rgb(187,44,162)">const</span> <span style="color:rgb(79,129,135)">lldb</span>::<span style="color:rgb(79,129,135)">DataExtractorSP</span> &data_sp);</div><div style="margin:0px;font-size:11px;font-family:Menlo;min-height:13px"><br></div><div style="margin:0px;font-size:11px;font-family:Menlo">    <span style="color:rgb(187,44,162)">void</span></div><div style="margin:0px;font-size:11px;font-family:Menlo">    SetOpaque (<span style="color:rgb(187,44,162)">const</span> <span style="color:rgb(79,129,135)">lldb</span>::<span style="color:rgb(79,129,135)">DataExtractorSP</span> &data_sp);</div></div><div><br></div><div>You could so something similar in your SBThreadCollection; provide protected accessors, constructor and setter, and only keep the default & copy constructors public</div><div><br></div><div>On a more philosophical note - is SBThreadCollection meant as a “read-only” snapshot of a set of threads as they existed at one time?</div><div>If so, then it’s all fine and dandy</div><div>But it seems that this class lacks any write access; I can’t create a new thread collection and start adding threads to it</div><div>Is this intended in your design, or did you just not feel the need to allow that?</div><div>Mostly curious..</div><div><br><div><blockquote type="cite"><div><div class="h5"><div>On Sep 5, 2014, at 2:20 PM, Kuba Brecka <<a href="mailto:kuba.brecka@gmail.com" target="_blank">kuba.brecka@gmail.com</a>> wrote:</div><br></div></div><div><div><div class="h5">Another piece of the puzzle, let's expose the newly abstracted ThreadCollection into SB API, based on <a href="http://reviews.llvm.org/D5200" target="_blank">http://reviews.llvm.org/D5200</a> and another patch that will actually provide an SBThreadCollection of ASan history threads is coming next.<br><br><a href="http://reviews.llvm.org/D5218" target="_blank">http://reviews.llvm.org/D5218</a><br><br>Files:<br>  include/lldb/API/SBDefines.h<br>  include/lldb/API/SBThreadCollection.h<br>  lldb.xcodeproj/project.pbxproj<br>  scripts/Python/build-swig-Python.sh<br>  scripts/Python/buildSwigPython.py<br>  scripts/Python/interface/SBThreadCollection.i<br>  scripts/lldb.swig<br>  source/API/SBThreadCollection.cpp<br></div></div><span><D5218.13339.patch></span>_______________________________________________<br>lldb-commits mailing list<br><a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br></div></blockquote></div><br><div>
<div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word"><div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word"><div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word"><div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word"><div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word"><div>Thanks,</div><div><i>- Enrico</i><br>📩 egranata@<font color="#ff2600"></font>.com ☎️ 27683</div><div><br></div></div></div></div></div></div><br><br>
</div>
<br></div></div></blockquote></div><br></div></div></div>