[cfe-dev] RFC redundant assignment in switch

[Daniel Marjamäki]

Hello!

I do know about that checker.

Personally I don't like that approach.

I don't like cluttering my code with comments that are not there to make the code easier to read/understand.

I don't think suppression comments should be required to avoid false positives.

But comments to avoid false negatives is ok/good. For instance annotations of utility functions.

What do you think about using this design goal in the static analyser?

Best regards,
Daniel Marjamäki

..................................................................................................................
Daniel Marjamäki Senior Engineer
Evidente ES East AB  Warfvinges väg 34  SE-112 51 Stockholm  Sweden

Mobile:                 +46 (0)709 12 42 62
E-mail:                 Daniel.Marjamaki at evidente.se

www.evidente.se

________________________________________
Från: Jordan Rose [jordan_rose at apple.com]
Skickat: den 28 mars 2014 21:52
Till: Daniel Marjamäki
Cc: cfe-dev at cs.uiuc.edu
Ämne: Re: [cfe-dev] RFC redundant assignment in switch

Hi, Daniel. We already have this in Clang itself as -Wimplicit-fallthrough, but it's only on in C++11 mode, where it can be silenced by adding "[[clang::fallthrough]];". There's a recent thread on cfe-dev about how to extend this to other language modes that don't have statement attributes; I'd suggest chiming in there.

Jordan


On Mar 28, 2014, at 6:33 , Daniel Marjamäki <Daniel.Marjamaki at evidente.se> wrote:

>
> Hello!
>
> I am working on a new checker for the StaticAnalyser.
>
> The purpose is to detect unintentional switch case fall through. The checker will warn when there is redundant assignment. Example code:
>
>    switch (x) {
>    case 1:
>       y = 1; // <- redundant assignment
>    case 2:
>       y = 2;
>       break;
>    }
>
> For your info, Cppcheck has this check. It has detected real bugs in real code. There are such cppcheck warnings in emacs,fvwm,etc. here is for example the code cppcheck warns about in fvwm:
>
>  case TEXTURE_BUILTIN:
>    Builtin:
>    TextureType=TEXTURE_BUILTIN;
>   default:
>    Solid:
>    TextureType=TEXTURE_SOLID;
>
> I would like to discuss the design.
>
> I attach a proof-of-concept checker.
>
> In the proof-of-concept code, variables that are assigned are tracked using a DenseMap. If there is no break and a variable is reassigned in the next case there will be a warning.
>
> I would like to know if I use the proper visitors and good approach for tracking assigned variables, etc.
>
> Best regards,
> Daniel Marjamäki
>
>
>
> ..................................................................................................................
> Daniel Marjamäki Senior Engineer
> Evidente ES East AB  Warfvinges väg 34  SE-112 51 Stockholm  Sweden
>
> Mobile:                 +46 (0)709 12 42 62
> E-mail:                 Daniel.Marjamaki at evidente.se
>
> www.evidente.se
> <redundantassignment1.patch>_______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev