<table><tr><td style="">brauch accepted this revision.<br />brauch added a comment.<br />This revision is now accepted and ready to land.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D14876">View Revision</a></tr></table><br /><div><div><p>Maybe you make the the visitAssignment code a bit more readable by breaking it up into 2-3 functions?</p>

<p>If you tend to the inline notes, this does not look too wrong ...</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D14876#inline-79199">View Inline</a><span style="color: #4b4d51; font-weight: bold;">declarationbuilder.cpp:1197</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                <span style="color: #aa4000">if</span> <span class="p">(</span> <span class="n">dec</span><span style="color: #aa2211">-></span><span class="n">kind</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">Declaration</span><span style="color: #aa2211">::</span><span class="n">Kind</span><span style="color: #aa2211">::</span><span class="n">Instance</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                    <span class="n">ClassMemberDeclaration</span> <span style="color: #aa2211">*</span><span class="n">classDec</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">dynamic_cast</span><span style="color: #aa2211"><</span><span class="n">ClassMemberDeclaration</span><span style="color: #aa2211">*></span><span class="p">(</span><span class="n">dec</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">move this down below the next condition, it doesn't need it</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D14876#inline-79201">View Inline</a><span style="color: #4b4d51; font-weight: bold;">declarationbuilder.cpp:1215</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">            <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">            <span class="n">lock</span><span class="p">.</span><span class="n">unlock</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I don't think you are allowed to unlock the lock here, you are still using the items below ... but in the builder code, stuff like this usually works because you have the parse lock in addition, so no idea.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D14876#inline-79203">View Inline</a><span style="color: #4b4d51; font-weight: bold;">declarationbuilder.cpp:1226</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                    <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">type</span> <span style="color: #aa2211">&&</span> <span class="n">type</span><span style="color: #aa2211">-></span><span class="n">dataType</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">IntegralType</span><span style="color: #aa2211">::</span><span class="n">TypeNull</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                        <span class="n">DUChainWriteLocker</span> <span class="n">wlock</span><span class="p">(</span><span class="n">DUChain</span><span style="color: #aa2211">::</span><span class="n">lock</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                        <span class="n">dec</span><span style="color: #aa2211">-></span><span class="n">setAbstractType</span><span class="p">(</span><span class="n">currentAbstractType</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">replace by: DUChainWriteLocker wlock;</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D14876#inline-79202">View Inline</a><span style="color: #4b4d51; font-weight: bold;">declarationbuilder.cpp:1228</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                        <span class="n">dec</span><span style="color: #aa2211">-></span><span class="n">setAbstractType</span><span class="p">(</span><span class="n">currentAbstractType</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                        <span class="n">wlock</span><span class="p">.</span><span class="n">unlock</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                    <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">remove this, it's unlocked automatically here anyways</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D14876#inline-79204">View Inline</a><span style="color: #4b4d51; font-weight: bold;">declarationbuilder.cpp:1248</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                <span class="n">lock</span><span class="p">.</span><span class="n">unlock</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">as above</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D14876#inline-79206">View Inline</a><span style="color: #4b4d51; font-weight: bold;">declarationbuilder.cpp:1270</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                            <span class="n">dec</span><span style="color: #aa2211">-></span><span class="n">setAbstractType</span><span class="p">(</span><span class="n">currentAbstractType</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                            <span class="n">wlock</span><span class="p">.</span><span class="n">unlock</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">                        <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">remove</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R52 KDevelop: PHP Support</div></div></div><br /><div><strong>BRANCH</strong><div><div>classes</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D14876">https://phabricator.kde.org/D14876</a></div></div><br /><div><strong>To: </strong>pprkut, brauch<br /><strong>Cc: </strong>brauch, kdevelop-devel, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd<br /></div>