Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Reporter
	Description • 15 years ago

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.

	Ehren Metcalfe 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?

	Tiziana Sellitto [:tiziana]
	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?

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me Reporter
	Comment 4 • 11 years ago

This area isn't being worked on and I'm not available to mentor in it.

	Joshua Cranmer [:jcranmer]
	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.