<table><tr><td style="">kossebau added a comment.
</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/D15694">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D15694#330761" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D15694#330761</a>, <a href="https://phabricator.kde.org/p/rjvbb/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@rjvbb</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>I'll point out that we're no longer living with Q_FOREACH and that nothing forbids anyone to provide a less conspicuous pattern than Q_FALLTHROUGH.</p></div>
</blockquote>
<p>Q_FALLTHROUGH has been picked up by many projects doing Qt-based software, also in codebase of KDE projects you already get > 100 hits (<a href="https://lxr.kde.org/search?_filestring=&_string=Q_FALLTHROUGH&_casesensitive=1" class="remarkup-link" target="_blank" rel="noreferrer">https://lxr.kde.org/search?_filestring=&_string=Q_FALLTHROUGH&_casesensitive=1</a>).</p>
<p>Using Q_FALLTHROUGH, even if ugly from purist POV (which I also share), as a temporary solution until C++17 is the base, should be fine and helpful for people jumping between Qt-based projects. One's eye gets used to it, I can tell :) As we also got used to Q_SLOTS and Q_SIGNALS..</p>
<p>My +1 for Q_FALLTHROUGH. It will be not used that frequently, so even more important to follow common naming.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">When case labels follow directly after others, no annotation is necessary.</pre></div></blockquote>
<p>I don't disagree, but it *is* an exception that you can easily extend to other cases where it's evident that a fallthrough is intended:</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">switch (type) {
case TYPE_A_OR_B:
do_something_for_a_or_b();
// it should be obvious that we're continuing straight to B</pre></div></blockquote>
<p>This "obvious" is where code readers and code writers usually disagree, and where good code gets some clarifying comment. It's one of the pain points with the switch statement for me, so I completely understand why compilers and C++ standards nudge into certain directions.<br />
A list of empty body cases (<tt style="background: #ebebeb; font-size: 13px;">case A: case B: case C:</tt>) is a widespread pattern and non-ambiguous. Once a case gets some statement, there is less pattern, so it's better to be explicit.</p>
<p>When it comes to no <tt style="background: #ebebeb; font-size: 13px;">break;</tt> after a <tt style="background: #ebebeb; font-size: 13px;">return;</tt>, that also matches the idea to not have an <tt style="background: #ebebeb; font-size: 13px;">else</tt> after an <tt style="background: #ebebeb; font-size: 13px;">return</tt> in the branch before. I have read lots of code, and skipping the <tt style="background: #ebebeb; font-size: 13px;">break;</tt> in that case does not ring bells for code reading confusion with me. Actually such lines with explicit unneeded break; only add noise to me, or worse, make think that line with the break; can still be reached somehow.<br />
So my +1 for the code-cleanup as done in this patch.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R32 KDevelop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D15694">https://phabricator.kde.org/D15694</a></div></div><br /><div><strong>To: </strong>aaronpuchert, KDevelop<br /><strong>Cc: </strong>rjvbb, brauch, mssola, kossebau, kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd<br /></div>