<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/106757/">http://git.reviewboard.kde.org/r/106757/</a>
     </td>
    </tr>
   </table>
   <br />








<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On October 7th, 2012, 5:51 p.m., <b>David Nolden</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  



<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="http://git.reviewboard.kde.org/r/106757/diff/1/?file=88669#file88669line669" style="color: black; font-weight: bold; text-decoration: underline;">languages/cpp/cppduchain/templatedeclaration.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">CppDUContext<KDevelop::DUContext>* instantiateDeclarationAndContext( KDevelop::DUContext* parentContext, const TopDUContext* source, KDevelop::DUContext* context, const InstantiationInformation& templateArguments, Declaration* instantiatedDeclaration, Declaration* instantiatedFrom, bool doNotRegister )</pre></td>

  </tr>
 </tbody>



 
 




 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">613</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">          <span class="n">AtomicIncrementer</span> <span class="n">safety</span><span class="p">(</span><span class="o">&</span><span class="n">alias_depth_counter</span><span class="p">);</span></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">619</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">          <span class="n">PushValue</span><span class="o"><</span><span class="n">uint</span><span class="o">></span><span class="p">(</span><span class="n">threadData</span><span class="p">.</span><span class="n">localData</span><span class="p">().</span><span class="n">aliasDepth</span><span class="p">,</span> <span class="o">+</span><span class="mi">1</span><span class="p">);</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You have to give PushValue a name, otherwise you have a no-op. ;-)</pre>
 </blockquote>





</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">thanks, fixed.

regarding performance, I ran this in kdevplatform source dir:
time CLEAR_DUCHAIN_DIR=1 duchainify --threads=2 .

with patch:
real    2m27.023s
user    2m32.973s
sys     0m3.866s

real    2m28.571s
user    2m33.823s
sys     0m3.960s

without patch:
real    2m26.959s
user    2m31.673s
sys     0m3.776s

real    2m26.225s
user    2m29.280s
sys     0m3.733s

So yeah, it indeed looks to be a bit slower. I'll investigate in-depth later this week or so, but I think I'll commit to master first anyways as the impact is not too bad (and this is just a 'time' »benchmark« anyways).</pre>
<br />




<p>- Milian</p>


<br />
<p>On October 7th, 2012, 4:50 p.m., Milian Wolff wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for KDevelop and David Nolden.</div>
<div>By Milian Wolff.</div>


<p style="color: grey;"><i>Updated Oct. 7, 2012, 4:50 p.m.</i></p>






<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Use a thread-local QMultiHash<IndexedQualifiedIdentifier, IndexedType> lookup table to support default template parameters. This obsoletes the necessity of the temporary DUContext which may introduce instabilities as described in bug 297133.

One big caveat was cloning template declarations which contain an internal context. This resulted in both, the original and clone to reference the same internal context. Upon destruction of the clone, the ownership of the internal context was tried to be changed. This can crash, as it could happen while only holding a read lock, yet the referenced internal context is in the DUChain and thus must only be altered while holding a write lock.

This change also makes it possible to remove the code surrounding Declaration::clone and Declaration::clonePrivate, as this was the only places which actually used it. Considering the major pitfalls and caveats of this API, I say this is a very good thing.

Furthermore, while introducing the thread-local data, I fixed the two safety recursion counters: Previously they where shared between threads which does not make any sense when we want to count the recursion depth that is thread-specific.</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">All existing unit tests pass. KDE Rocs and boost/proto/matches.hpp get parsed without crashing.</pre>
  </td>
 </tr>
</table>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="http://bugs.kde.org/show_bug.cgi?id=297133">297133</a>


</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>languages/cpp/cppduchain/templatedeclaration.cpp <span style="color: grey">(e423693)</span></li>

 <li>languages/cpp/cppduchain/templateparameterdeclaration.h <span style="color: grey">(9fb44aa)</span></li>

 <li>languages/cpp/cppduchain/templateparameterdeclaration.cpp <span style="color: grey">(08b76bb)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/106757/diff/" style="margin-left: 3em;">View Diff</a></p>




  </td>
 </tr>
</table>








  </div>
 </body>
</html>