C++ coding style inconsistencies

[Richard Biener]

On Thu, Jun 25, 2015 at 10:44 PM, Jeff Law <law@redhat.com> wrote:
> On 06/25/2015 12:28 PM, Richard Sandiford wrote:
>>
>> Sorry in advance for inviting a bikeshed discussion, but while making
>> the hashing changes that I just committed, I noticed that the C++ification
>> has been done in a variety of different styles.  I ended up having to
>> follow
>> the "do what the surrounding code does" principle that some code bases
>> have,
>> but to me that's always seemed like an admission of failure.  One of the
>> strengths of the GCC code base was always that it was written in a very
>> consistent style.  Regardless of what you think of that style (I
>> personally
>> like it, but I know others don't at all), it was always easy to work on
>> a new area of the compiler without having to learn how the surrounding
>> code
>> preferred to format things.  It would be a shame if we lost that in the
>> rush to make everything "more C++".
>>
>> The three main inconsistencies I saw were:
>>
>> (1) Should inline member functions be implemented inside the class or
>> outside
>>      the class?  If inside, should they be formatted like this:
>>
>>         void
>>         foo (args...)
>>         {
>>           ...;
>>         }
>>
>>      or like this:
>>
>>         void
>>         foo (args...)
>>           {
>>             ...;
>>           }
>>
>>      (both have been used).
>>
>>      The coding standard is pretty clear about this one:
>>
>>          Define all members outside the class definition. That is, there
>>          are no function bodies or member initializers inside the class
>>          definition.
>>
>>      But in-class definitions have become very common.  Do we want
>>      to revisit this?  Or do we just need more awareness of what the
>>      rule is supposed to be?
>>
>>      [Personally I like the rule.  The danger with in-class definitions
>>      is that it becomes very hard to see the interface at a glance.
>>      It obviously makes things more verbose though.]
>
> I'd say let's go with the existing rule.  I know that in-class definitions
> are relatively common, particularly if they are trivial, so do we want an
> exception for anything that fits on a single line?

I think so.  Similar for constructors with only : initializers and otherwise
empty body.

Richard.

>
>>
>> (2) Is there supposed to be a space before a template parameter list?
>>      I.e. is it:
>>
>>         foo<bar>
>>
>>      or:
>>
>>         foo <bar>
>>
>>      ?  Both are widely used.
>>
>>      The current coding conventions don't say explicitly, but all the
>>      examples use the second style.  It's also more in keeping with
>>      convention for function parameters.  On the other hand, it could
>>      be argued that the space in:
>>
>>         foo <bar, frob>::thing
>>
>>      makes the binding confusing and looks silly compared to:
>>
>>         foo<bar, frob>::thing
>>
>>      But there again, the second one might look like two unrelated
>>      blobs at first glance.
>
> I'd go with whatever gnu-ident does with these things.  I'd hate for us to
> settle on a style that requires non-default behaviour from gnu-indent.
>
>
>
>
>
>>
>> (3) Do we allow non-const references to be passed and returned by
>>      non-operator functions?  Some review comments have pushed back
>>      on that, but some uses have crept in.
>>
>>      [IMO non-const references are too easy to misread as normal
>>      parameters.]
>>
>> In all three cases, whether the answer is A or B is less important
>> than whether the answer is the same across the code base.
>
> I could make an argument either way on this one...
>
> jeff
>