C++ coding style inconsistencies

[Richard Sandiford <rdsandiford at googlemail dot com>]

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.]

(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.

(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.

If this message does generate any discussion, I'm happy to write up
the result in the coding conventions and try to make the code base
consistent with it.

Thanks,
Richard