List:Internals« Previous MessageNext Message »
From:Davi Arnaut Date:September 26 2010 2:35am
Subject:Re: [STYLE] could we allow // for comments alone on their line?
View as plain text  
On 9/25/10 5:07 PM, Mats Kindahl wrote:
> On 09/25/2010 03:31 AM, Davi Arnaut wrote:
>> On 9/24/10 12:22 PM, Mats Kindahl wrote:
>>> In this particular case, we would probably be better off if we could
>>> turn on -std=c89, which disallows C++ comments in C code.
>>>
>>> The downside is that it would of course mean that we have to start
>>> writing standards-compliant C code (oh horrors!). :)
>>
>> I wish that overtime we could focus more on styles and actions, such
>> as this, that have more practical consequences. Over time, discussions
>> about the cosmetic aspects of coding style tend to get very silly.
>
> Ah, yes... :)
>
>> How about a proposal that new C and C++ code must be
>> standards-compliant? That it must compile warning free? etc.
>
> Oh, I would love that, but I wonder how that can be handled practically.
> New code in separate files? Rewriting the files one by one?

Fixing issues one by one.

> It would be great if -ansi or -Werror could be enabled on a per-file basis.

We already do -Werror for all files. See the maintainer mode option:

C_WARNINGS "-Wall -Wextra -Wunused -Wwrite-strings -Werror"
CXX_WARNINGS "${MY_MAINTAINER_C_WARNINGS} -Wno-unused-parameter"

but this side steps the strict aliasing warnings as we have too many 
violations of it. It will take some time to fix them all.

I'm planning to submit a style request that we enable the maintainer 
mode by default for debug builds.

>> Do you have an idea of how much work is needed to make MySQL compile
>> with -std=c89?
>
> Sadly, yes. I did make an attempt to rewrite some aspects I disliked
> (the cast of string literals to "char*", it didn't look like much work),
> but it turned out to ripple through the code. If we could handle it on a
> one-file-at-a-time basis, I think it could be done.

Actually, I just tested HAVE_CMAKE=no 
BUILD/compile-pentium-valgrind-max-no-ndb --warning-mode=pedantic in 5.5 
and it built fine after fixing one case of C++ style comment in C file. 
It builds C files with -ansi -pedantic and C++ ones also with -std=c++98.

But there were quite a few warnings. I guess the challenge is to get it 
working without warnings (-Werror). For this we would need proper 
configure time checks for both cmake (like, mapping inline to 
__inline__, likewise for asm) and working around the silly stuff such as 
%p requiring the argument to be of type void pointer (non-standard, but..).

Will take a more detailed look at the warnings later. Anyway, quite nice 
that it actually compiles successfully.

Regards,

Davi
Thread
[STYLE] could we allow // for comments alone on their line?Guilhem Bichot23 Sep
  • Re: [STYLE] could we allow // for comments alone on their line?Joerg Bruehe24 Sep
    • Re: [STYLE] could we allow // for comments alone on their line?Mats Kindahl24 Sep
      • Re: [STYLE] could we allow // for comments alone on their line?Mark Leith24 Sep
      • Re: [STYLE] could we allow // for comments alone on their line?Davi Arnaut25 Sep
        • Re: [STYLE] could we allow // for comments alone on their line?Mats Kindahl25 Sep
          • Re: [STYLE] could we allow // for comments alone on their line?Davi Arnaut26 Sep
        • Re: [STYLE] could we allow // for comments alone on their line?Guilhem Bichot27 Sep
          • Re: [STYLE] could we allow // for comments alone on their line?Martin Hansson28 Sep
          • Re: [STYLE] could we allow // for comments alone on their line?Davi Arnaut28 Sep
  • Re: [STYLE] could we allow // for comments alone on their line?Ingo Struewing24 Sep
  • [STYLE] could we allow // for comments alone on their line?Michael Widenius30 Sep