[vtk-developers] coding standards & switch case label accidental fallthrough

[Andrew Maclean]

Sean,

I don't see the need for introducing yet another VTK_ macro for what is a
rare and perfectly legal code construct that is picked up as a warning by
one compiler type.

You are just imposing another constraint on programmers for what is a
perfectly legal construct.

What warning level are you using? I don't think that I have seen this
warning when building VTK.

As David Lonie says it does have its uses.

Even when looking at Common/DataModel/vtkHyperTreeGrid.cxx in your link
below I think it is easier to read and understand without the added macro
(line 1108 onwards). Similarly in a few of the other ones
e.g. vtkImageMapToColors.cxx, vtkImageInterpolator.cxx that you have there.

Having said this I am amazed at the number of missing break statements that
you have found. It is a wonder some of this code ever worked! So the
warning is good in that respect.

On a more general vein I deeply appreciate the effort you have put into
cleaning up VTK over the past year or so. The code base is so much better
thanks to your efforts.

What do others think?

Regards
   Andrew



> ---------- Forwarded message ----------
> From: David Lonie <david.lonie at kitware.com>
> To: Sean McBride <sean at rogue-research.com>
> Cc: vtkdev <vtk-developers at vtk.org>
> Date: Wed, 8 Apr 2015 09:49:23 -0400
> Subject: Re: [vtk-developers] coding standards & switch case label
> accidental fallthrough
> +1 for the change. The explicit token is great for readability and will be
> important as other compilers start warning about these.
>
> -1 for forbidding them. A lot of times it just makes the most sense to use
> a fall through, as long as it's clearly labeled as intentional.
>
> On Tue, Apr 7, 2015 at 1:49 PM, Sean McBride <sean at rogue-research.com>
> wrote:
>
>> On Tue, 7 Apr 2015 13:42:51 -0400, David Cole said:
>>
>> >Is there really no way to refactor and simply eliminate all the
>> >"intentional" fall throughs? It's a really nasty style to be allowing.
>> >It should be discouraged in general.
>>
>> I did refactor a few, but most I left because I was scared to touch or
>> they were pretty elegant with fall-through.  You can take a look:
>>  <https://gitlab.kitware.com/vtk/vtk/merge_requests/59>
>>
>> I'd be in favour of the coding standards saying something like
>> "fall-through is discouraged, but when necessary must be annotated with
>> VTK_FALLTHROUGH".
>>
>> Cheers,
>>
>> --
>> ____________________________________________________________
>> Sean McBride, B. Eng                 sean at rogue-research.com
>> Rogue Research                        www.rogue-research.com
>> Mac Software Developer              Montréal, Québec, Canada
>>
>>
>> _______________________________________________
>> Powered by www.kitware.com
>>
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html
>>
>> Search the list archives at: http://markmail.org/search/?q=vtk-developers
>>
>> Follow this link to subscribe/unsubscribe:
>> http://public.kitware.com/mailman/listinfo/vtk-developers
>>
>>
>

-- 
___________________________________________
Andrew J. P. Maclean

___________________________________________
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20150409/06c6b7fb/attachment.html>