1288036 - [Static Analysis][Clang-Plugin] Missing break in switch statements

Status: RESOLVED DUPLICATE of bug 1253170

(Developer Infrastructure :: Source Code Analysis, defect, P3)

Description

[Andi [:andi]]

We must have a checker that detects when a case statement is not ended by a break or even return. I think the case that we would like to check is similar to:

>>  switch (type) {
>>    TYPE_1:
>>    TYPE_2: {
>>      // do something
>>      break;
>>    }
>>    TYPE_3:
>>      // do something else
>>      
>>    TYPE_4:
>>    TYPE_5:
>>    TYPE_6: {
>>      // other instructions
>>      return;
>>    }
>>  }

Maybe for case TYPE_3 we should address a warning message since it could lead to a potential side effect. So the idea is that if we encounter children for a LabelStmt that do not end with a BreakStmt or ReturnStmt we should address a warning message. I wouldn't add an error since in some particular cases this is actually the desired behaviour.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Why not try to see if we can get firefox to build with the built-in-to-clang analysis for this: -Wno-implicit-fallthrough I think.

Comment 2

[Andi [:andi]]

(In reply to Michael Layzell [:mystor] from comment #1)
> Why not try to see if we can get firefox to build with the built-in-to-clang
> analysis for this: -Wno-implicit-fallthrough I think.

This is a good idea, also this kind of checker is prone to give a high yield of false-positive.

Comment 3

[Chris Peterson [:cpeterson]]

I think this bug is a duplicate of bug 1253170, which enabled clang's -Wimplicit-fallthrough warnings by default. Intentional fallthroughs can be annotated with the `MOZ_FALLTHROUGH;` macro (bug 1215411).