[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

I know this has been reverted, but for the record, the implementation has a few shortcomings that makes it hard to use it for projects that rely on fallthrough comments:

- The comment is not recognized if it is on the same line as the last statement, e.g.

  case 2:
    n++; // fallthrough
  case 3:
    // ...

- The implementation only scans until the next non-empty line and checks it for a comment, so there can't be a comment between the fallthrough comment and the last statement, making the following case not work:

  case 2:
    if (some_bool)
      n++;
    // otherwise, don't increment n
  
  // fallthrough
  case 3:
    // ...

- If the last statement ends in a macro, this implementation checks the source code at the definition of the macro, so this breaks:

  #define SOME_MACRO 1
  
  switch(c) {
    case 0:
      n += SOME_MACRO;
   //fallthrough
    case 1:
    // ...
  }

- I'm not 100% why this is the case, but for more complex if statements, the second branch will start the comment search in the line of the if statement:

  case 0:
    if (n == 3 && p == 4)
      n++;
  
  // fallthrough
  case1:
    //...

They aren't hard to fix, but I think it would generally make more sense to search for a fallthrough comment upwards from a case statement instead of downwards. That way the "pick the first non-empty line matching the regex" logic should work, too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73852/new/

https://reviews.llvm.org/D73852