D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks

Friedrich W. H. Kossebau noreply at phabricator.kde.org
Sun Sep 23 21:25:15 BST 2018


kossebau added a comment.


  In D15694#330761 <https://phabricator.kde.org/D15694#330761>, @rjvbb wrote:
  
  > 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.
  
  
  Q_FALLTHROUGH has been picked up by many projects doing Qt-based software, also in codebase of KDE projects you already get > 100 hits (https://lxr.kde.org/search?_filestring=&_string=Q_FALLTHROUGH&_casesensitive=1).
  
  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..
  
  My +1 for Q_FALLTHROUGH. It will be not used that frequently, so even more important to follow common naming.
  
  >>   When case labels follow directly after others, no annotation is necessary.
  > 
  > 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:
  > 
  >   switch (type) {
  >       case TYPE_A_OR_B:
  >          do_something_for_a_or_b();
  >          // it should be obvious that we're continuing straight to B
  
  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.
  A list of empty body cases (`case A: case B: case C:`) is a widespread pattern and non-ambiguous. Once a case gets some statement, there is less pattern, so it's better to be explicit.
  
  When it comes to no `break;` after a `return;`, that also matches the idea to not have an `else` after an `return` in the branch before. I have read lots of code, and skipping the `break;` 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.
  So my +1 for the code-cleanup as done in this patch.

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D15694

To: aaronpuchert, #kdevelop
Cc: rjvbb, brauch, mssola, kossebau, kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20180923/960c9f9b/attachment.html>


More information about the KDevelop-devel mailing list