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

[Aaron Puchert]

aaronpuchert added a comment.


  In D15694#330687 <https://phabricator.kde.org/D15694#330687>, @rjvbb wrote:
  
  > As mentioned on another ticket, I don't like the idea of not putting `break`s, no matter how clever the compiler and hard-coded choice of compiler options.
  
  
  The compiler doesn't need to be clever, it has to understand control flow anyway to compile the program. With Clang, you can actually see the generated control flow graph of your source file with the options `--analyze -Xanalyzer -analyzer-checker=debug.DumpCFG`.
  
  > I don't think you gain anything from it, and it doesn't improve code readability either for me
  
  Can you explain in more detail how an unreachable `break` after a `return` statement makes the code more readable? Actually #framework_syntax_highlighting <https://phabricator.kde.org/tag/framework_syntax_highlighting/> distinguishes between control flow keywords and other keywords. Depending on your color scheme, you'll see if there is a terminator or not.
  
  > Making it obligatory to add a token confirming that fallthrough is intended is a good idea, though using a largish term in all-caps like `Q_FALLTHROUGH` also doesn't improve readibilty.
  
  With C++17 you can use `[[fallthrough]];` — until then we have to live with the all-caps. Apart from that, fallthroughs are not very common, and one could argue that they should be drawing a reader's attention.
  
  > And what about the most common type of fallthrough, are we really going to have to write something like the following?
  
  When case labels follow directly after others, no annotation is necessary. Only when there is another statement in between. That doesn't happen very often, and if it does, the annotation certainly doesn't hurt.
  
    switch (type) {
        case TYPE_A: // no fallthrough annotation needed
        case TYPE_B: // no fallthrough annotation needed
        case TYPE_C: // no fallthrough annotation needed
        case TYPE_D:
            whatever();
            break;  // annotation would be necessary if we didn't break
        default:
            anyway();
            // break optional
    }

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/2774d333/attachment-0001.html>