llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.97k stars 11.94k forks source link

[clang] False positive -Wimplicit-fallthrough for fallthrough to break #102678

Open chrchr-github opened 3 months ago

chrchr-github commented 3 months ago
int f(int i) {
    int j = 0;
    switch (i) {
    case 1: {
        j = 2;
    }
    default:
        break;
    }
    return j;
}
<source>:7:9: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
    7 |         default:
      |         ^
<source>:7:9: note: insert 'break;' to avoid fall-through
    7 |         default:
      |         ^
      |         break; 

https://godbolt.org/z/PezPf5Pn5

philnik777 commented 2 months ago

I don't think that this is a false-positive. Whether the next branch is doing anything doesn't really change whether it's a fall-through or not, and it's still most likely not intended by the author. You even say in the title that it's a fall-through.

chrchr-github commented 2 months ago

Well, the warning is technically correct. It just doesn't improve the code much to insert a break to avoid falling through to a break.

philnik777 commented 2 months ago

Well, the warning is technically correct. It just doesn't improve the code much to insert a break to avoid falling through to a break.

I guess that's where we disagree. IMO there should either be a [[fallthrough]] or break; to make it clear what the intention was, since it's not clear to me currently. In this case it doesn't make a difference, but the default: break; doesn't either. In a longer switch I'd definitely want to make it clear.

shafik commented 2 months ago

I agree, I don't think this is a bug. Explicitly indicating fallthrough is about annotating intention.