<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="https://git.reviewboard.kde.org/r/115383/">https://git.reviewboard.kde.org/r/115383/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">besides the lock issue below - good work!</pre>
<br />
<div>
<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="https://git.reviewboard.kde.org/r/115383/diff/2/?file=241048#file241048line179" style="color: black; font-weight: bold; text-decoration: underline;">language/codegen/basicrefactoring.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">void BasicRefactoring::startInteractiveRename(const KDevelop::IndexedDeclaration &decl)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">179</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">NameAndCollector</span> <span class="n">nc</span> <span class="o">=</span> <span class="n">newNameForDeclaration</span><span class="p">(</span><span class="n">declaration</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">considering you acquire the lock in here anyways, keep it while calling it and also add to the apidox that newNameForDeclaration should be called while holding the duchain lock.
Then remove the DUChainReadLocker from the newNameForDeclaration.
Also, note that otherwise this code is potentially crashy as the Declaration* might get deleted between lock.unlock() above and DUChainReadLocker lock; in newNameForDeclaration. To guard against that you'd need to use DeclarationPointer - or keep the lock alive.</pre>
</div>
<br />
<p>- Milian Wolff</p>
<br />
<p>On January 30th, 2014, 8:42 a.m. UTC, Sergey Kalinichev wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDevelop.</div>
<div>By Sergey Kalinichev.</div>
<p style="color: grey;"><i>Updated Jan. 30, 2014, 8:42 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevplatform
</div>
<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;">Refactore code out a little bit so that language plugins can reuse even more code if they want to reimplement startInteractiveRename function.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>language/duchain/navigation/useswidget.h <span style="color: grey">(f52d868)</span></li>
<li>language/codegen/basicrefactoring.cpp <span style="color: grey">(d5e8c67)</span></li>
<li>language/codegen/basicrefactoring.h <span style="color: grey">(06606a5)</span></li>
<li>language/duchain/navigation/useswidget.cpp <span style="color: grey">(2fd0bd9)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115383/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>