[cfe-commits] [PATCH] Implicit fall-through between switch labels

[David Blaikie]

Hi Alex,

Nice diagnostic - here's a few bits of feedback:

* remove commented out logging/debug code in DiagnoseMissingBreaks
* the diagnostic message seems overly specific - there are other ways
to exit (continue or throw, for example). I'm not sure if there's a
nice way to express this so we can avoid enumerating all these options
  * I'm not sure if you want/need test cases for throw/continue - I
verified they work, but really many of the cases you have or could add
are just features of the CFG builder (& are probably already tested by
the missing return diagnostic, or other such things)
* do we have precedent for mentioning the name of an attribute in a
diagnostic? using the [[]] seems a little off to me, but I'm not sure
saying "use the fallthrough attribute" is terribly helpful.
* Is there some generic utility we do/could have for testing whether
an attributable entity has a certain attribute applied to it? (that
you could use to generalize/instead of ContainsFallThrough)
* I would be inclined to have something about cases, fallthrough, or
switches in the name of the "DiagnoseMissingBreaks" function - though
I appreciate it's close to the related code & file local. Not a big
deal.
* Do you have any sense of the current quality of this diagnostic?
It's probably OK to go in as-is and off-by-default & then see whether
we need to make any improvements to it.

Just some food for thought.

- David

On Mon, Apr 16, 2012 at 8:40 AM, Alexander Kornienko <alexfh at google.com> wrote:
> Oops, you're right. Here it is.
>
> On Mon, Apr 16, 2012 at 5:27 PM, Dmitri Gribenko <gribozavr at gmail.com>
> wrote:
>>
>> On Mon, Apr 16, 2012 at 6:17 PM, Alexander Kornienko <alexfh at google.com>
>> wrote:
>> > Hi all,
>>
>> Hi Alexander, [off-list]
>>
>> It seems like you didn't attach the patch.
>>
>> Dmitri
>>
>> --
>> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
>> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>