Closed
Bug 522774
Opened 15 years ago
Closed 11 years ago
Static analysis: Error when falling though switch cases without an explicit "fallthrough" annotation
Categories
(Developer Infrastructure :: Source Code Analysis, enhancement)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: cjones, Assigned: ehren.m)
References
Details
Attachments
(2 files)
I've coded this bug all the time switch(foo) { case FOO: doSomething(); // oops! forgot to |break;| case BAR: doSomethingElse(); // CRASH! bad fallthrough } It would be great to require an explicit annotation for falling through switch cases. Perhaps something like __attribute__(((user "fallthrough"))) inline static fallthrough() { } #define FALLTHROUGH() fallthrough() switch (foo) case FOO: doSomething(); // if i wanted to fallthrough, i would write // FALLTHROUGH(); case BAR: ... } The code above would trigger an error, because the fallthrough wasn't explicitly allowed. Uncommenting "// FALLTHROUGH();" would suppress the error. This is a pretty straightforward treehydra-CFG analysis.
Reporter | ||
Updated•15 years ago
|
Whiteboard: [good first bug]
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → ehren.m
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•15 years ago
|
||
Here's a standalone script that's pretty much airtight. The only 'false positive' is |case '\0'| here: http://mxr.mozilla.org/mozilla-central/source/js/src/jskwgen.cpp#213 but this is kind of a fallthrough in disguise. Also, whenever a case falls through to more than one case eg case 0: x++ /* Fallthrough */ case 1: case 2: case 3: x--; the script will issue multiple warnings, but I suppose that's not too much of an issue. If we were willing to go forward and add this to a static checking build, would adding the 'fallthrough function' to nscore.h be sufficient?
Assignee | ||
Comment 2•15 years ago
|
||
Comment 3•11 years ago
|
||
I'm going through old "good first bugs" that have been inactive for a while. Chris, is this something you can help Ehren with? Do you think this bug could become a mentored bug?
Reporter | ||
Comment 4•11 years ago
|
||
This area isn't being worked on and I'm not available to mentor in it.
Whiteboard: [good first bug]
Comment 5•11 years ago
|
||
Our static analysis is built off of Clang these days, which has a warning for this sort of stuff (-Wimplicit-fallthrough) and an attribute to mark when it is explicitly desired ([[clang::fallthrough]]). We don't need a static analysis for this anymore, then.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
Updated•7 years ago
|
Product: Core → Firefox Build System
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•