On 2016.09.27 at 12:56 +0200, Marek Polacek wrote:
> On Tue, Sep 27, 2016 at 12:47:50PM +0200, Jakub Jelinek wrote:
> > On Tue, Sep 27, 2016 at 12:39:41PM +0200, Markus Trippelsdorf wrote:
> > > On 2016.09.27 at 10:46 +0200, Eric Botcazou wrote:
> > > > > The intent has been that we catch the most common forms, but still 
> > > > > require
> > > > > it not to be complete free form.  Because, as experience shows, 
> > > > > people are
> > > > > extremely creative in these comments, and it is not very good idea to
> > > > > support everything.  For ... fall through ... , what is the purpose of
> > > > > those ...s?
> > > >
> > > > No idea, but it has been there for a while and seems perfectly 
> > > > reasonable.
> > > > IMO any sentence containing "fall" and "through/thru/etc" on the same 
> > > > line
> > > > should be accepted, otherwise it's just misplaced pickiness.
> > >
> > > +1. Folks will just disable the warning if gcc is not very permissive
> > > when paring existing comments. You cannot expect anyone to change
> > > perfectly fine fall-through comments just to accommodate an arbitrary
> > > gcc style.
> >
> > The accepted style is already very permissive, we don't allow just one
> > spelling as various lint tools.  I'm afraid looking for various cases of
> > fall and through/thru possibly separated by anything and surrounded by
> > anything is IMHO already too much, the compiler shouldn't try to try to
> > grammar analyze the comments on what they actually talk about and whether it
> > might be related to the switch fall through or something completely
> > different.  Users should start using [[fallthrough]]; anyway.
>
> I'm thinking perhaps we should also accept /* ... fall through ... */
> and /* else fall through */, but accepting any sentence containing "fall" and
> "through/thru/etc" on the same line would mean that we also accept
> /* Don't fall through here.  */ and that is clearly not desirable.
>

I'm also wondering about the situation where not a single break is used
in all of the cases. It would be best not to warn here.

An example from ffmpeg:

#define LPC1(x) {           \
    int c = coefs[(x)-1];   \
    p0   += MUL(c, s);      \
    s     = smp[i-(x)+1];   \
    p1   += MUL(c, s);      \
}

static av_always_inline void FUNC(lpc_encode_unrolled)(int32_t *res,
                                  const int32_t *smp, int len, int order,
                                  const int32_t *coefs, int shift, int big)
{
    int i;
    for (i = order; i < len; i += 2) {
        int s  = smp[i-order];
        sum_type p0 = 0, p1 = 0;
        if (big) {
            switch (order) {
            case 32: LPC1(32)
            case 31: LPC1(31)
            case 30: LPC1(30)
            case 29: LPC1(29)
            case 28: LPC1(28)
            case 27: LPC1(27)
            case 26: LPC1(26)
            case 25: LPC1(25)
            case 24: LPC1(24)
            case 23: LPC1(23)
            case 22: LPC1(22)
            case 21: LPC1(21)
            case 20: LPC1(20)
            case 19: LPC1(19)
            case 18: LPC1(18)
            case 17: LPC1(17)
            case 16: LPC1(16)
            case 15: LPC1(15)
            case 14: LPC1(14)
            case 13: LPC1(13)
            case 12: LPC1(12)
            case 11: LPC1(11)
            case 10: LPC1(10)
            case  9: LPC1( 9)
                     LPC1( 8)
                     LPC1( 7)
                     LPC1( 6)
                     LPC1( 5)
                     LPC1( 4)
                     LPC1( 3)
                     LPC1( 2)
                     LPC1( 1)
            }
        } else {
            switch (order) {
            case  8: LPC1( 8)
            case  7: LPC1( 7)
            case  6: LPC1( 6)
            case  5: LPC1( 5)
            case  4: LPC1( 4)
            case  3: LPC1( 3)
            case  2: LPC1( 2)
            case  1: LPC1( 1)
            }
        }
        res[i  ] = smp[i  ] - CLIP(p0 >> shift);
        res[i+1] = smp[i+1] - CLIP(p1 >> shift);
    }
}


--
Markus

Reply via email to