Re: [RFA 1/3] Fix "fall through" comments

[Yao Qi <qiyaoltc at gmail dot com>]

On Wed, Sep 28, 2016 at 9:12 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
>
> Tom> On my machine, clang won't build gdb.  From memory there was at
> Tom> least some issue compiling .c files in c++ mode, but I think there
> Tom> were other problems as well.
>
> Yao> We can use option '-x c++' to suppress the warnings, but you are
> Yao> right, there are some other problems, like some warning options are
> Yao> unknown to clang.
>
> Yeah.  I tried it again and there are two issues.
>
> First, clang++ doesn't like the .c extension:
>
>     clang-3.8: error: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated
>
> I was able to fix this with:
>
>     make CC=clang CXX='clang++ -x c++' CC_LD=clang++
>
> After that I got:
>
>     error: unknown warning option '-Wunused-but-set-parameter'; did you mean '-Wunused-parameter'? [-Werror,-Wunknown-warning-option]
>     error: unknown warning option '-Wunused-but-set-variable'; did you mean '-Wunused-const-variable'? [-Werror,-Wunknown-warning-option]
>
> This is probably an oversight in warning.m4, as in config.log I see:
>
>     configure:14316: clang++ -c -g -O2 -Wunused-but-set-parameter  conftest.cpp >&5
>     warning: unknown warning option '-Wunused-but-set-parameter'; did you mean '-Wunused-parameter'? [-Wunknown-warning-option]
>     1 warning generated.
>     configure:14316: $? = 0
>
> Adding 'WERROR_CFLAGS=' solves this... but at this point it's fine to
> keep fall-through comments, because warnings are disabled anyway.
>
> It's worth noting that I see tons of "unused function" warnings from
> vec.h. when building this way.  There's definitely some work to be done
> here if anybody wants to use clang.
>

Yes, I see all of them above in my build.

> In sum my view is that it's fine to go ahead with the comment approach.

OK.

> If someone wants to fix up the clang build I can help convert the
> comments to an attribute.  (The issue with doing it up-front is
> discovering the spots that would need a change -- gcc won't tell us what
> they are.)
>

Agreed.  Your patch is good to me then.

-- 
Yao (齐尧)