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

[Simon Marchi <simon.marchi@polymtl.ca>]

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