<table><tr><td style="">ervin requested changes to this revision.<br />ervin added a comment.<br />This revision now requires changes to proceed.
</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/D26129">View Revision</a></tr></table><br /><div><div><p>The patch being large I didn't check each one separately but I suspect this patch misses boat loads of qAsConst. Besides there's at least one case of very flawed logic, it can't be about blindly switching one loop construct for another especially when the iterators are compared inside the loop body.</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/D26129#inline-147484">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kconfig_compiler.cpp:2119</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; ">                <span class="n">h</span> <span style="color: #aa2211"><<</span> <span class="n">type</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">" "</span> <span style="color: #aa2211"><<</span> <span class="n">argument</span><span class="p">.</span><span class="n">variableName</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">                <span style="color: #aa4000">if</span> <span class="p">(<span class="bright"></span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">++</span></span><span class="bright"></span><span class="n"><span class="bright">i</span>t</span> <span style="color: #aa2211">!=</span> <span class="bright"></span><span class="n"><span class="bright">itEnd</span></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 style="color: #aa4000">if</span> <span class="p">(<span class="bright"></span></span><span class="bright"></span><span class="n"><span class="bright">argumen</span>t</span> <span style="color: #aa2211">!=</span> <span class="bright"></span><span class="n"><span class="bright">signal</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">arguments</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">last</span></span><span class="bright"></span><span class="p"><span class="bright">()</span>)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">                    <span class="n">h</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">", "</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This logic is very flawed... instead of comparing positions in the list now you're comparing actual values. If two different positions in the list would end up having the same values (for some reason) we'd generate syntactically incorrect code.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R237 KConfig</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D26129">https://phabricator.kde.org/D26129</a></div></div><br /><div><strong>To: </strong>tcanabrava, ervin<br /><strong>Cc: </strong>ervin, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns<br /></div>