Re: [PATCH 0/5] Remove a few hurdles of compiling with clang

Simon Marchi
2017-06-12T16:44:26Z

On 2017-06-12 18:23, Andrew Pinski wrote:
> On Mon, Jun 12, 2017 at 8:54 AM, Simon Marchi <simon.marchi@polymtl.ca>
> wrote:
>> - gdb: Pass -x c++ to the compiler: GCC (and even the Intel compiler)
>> supports this option too, at worst it's a neutral change for compiling
>> with
>> GCC.
>
> Why is this needed? Why can't you use clang++ or similar to force
> compiling as C++?

Sorry for being unclear. I do use clang++ and it complains:

$ clang++ test.c
clang: warning: treating 'c' input as 'c++' when in C++ mode, this
behavior is deprecated

So using -x c++ is the only way I found to make it work (short of
renaming the files).

>> - gdb: Use -Werror when checking for (un)supported warning flags: it
>> just
>> forces the behavior to what's already the default with GCC. Again,
>> it's
>> neutral at worst, at best it protects us if GCC ever decides to change
>> its
>> default behavior.
>
>
> Yes I think this is ok because gcc also does not warn about
> unsupported warning flags unless there is an error.

Hmm that's not what I observe:

$ gcc test.c
$ gcc test.c -Wfoo
gcc: error: unrecognized command line option ‘-Wfoo’

>> - gdb: Add -Wno-mismatched-tags: We already have a system that tests
>> which
>> warning flags are supported by the current compiler, so this flag will
>> not
>> be included in the builds with GCC. So it's neutral for GCC, and
>> improves
>> the situation for Clang with almost no effort.
>
> This warning is a bug in clang and really should not be warned about
> in either -Wall or -Wextra. I have been complaining about this since
> clang added this option.

>>
>> - linux-low: Remove usage of "register" keyword: That's a good legacy
>> code
>> cleanup in any case, IMHO.
>
> Yes and no. I don't think register was deprecated in C++11.

From what I understand, the register keyword has more or less no effect
on compilers today, so it would be pretty much useless. So we can
remove it and we don't really care.

>>
>> - Add ATTRIBUTE_PRINTF to trace_start_error: It's actually a legit
>> warning,
>> I'm surprised GCC itself doesn't warn about that.
>
> This warning does not make sense. Can you give some more context?

Sure, the actual error is:

/home/emaisin/src/binutils-gdb/gdb/gdbserver/../nat/fork-inferior.c:582:13:
error: format string is not a string literal
[-Werror,-Wformat-nonliteral]
vwarning (fmt, ap);
^~~
1 error generated.

That code is at:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/nat/fork-inferior.c;h=0913409e6dd01e78019e85068f2e34e8e744ec5b;hb=HEAD#l575

The vwarning function is itself marked with ATTRIBUTE_PRINTF:

31 extern void vwarning (const char *fmt, va_list args)
32 ATTRIBUTE_PRINTF (1, 0);

So it makes sense (I think) to complain that the value passed to fmt is
not a literal, unless that value is also marked as being a format
string. Then it pushes the requirement of passing a literal to the
caller.

Simon