RFC: 8247976: Update HotSpot Style Guide for C++14 adoption

[Thomas Stüfe]

(note: original mail was lost to us because of the hotspot-dev outage, so I
am answering to what I read in the ML archive:
https://mail.openjdk.java.net/pipermail/hotspot-dev/2020-June/042247.html )

Hi Kim,

Thank you for your work, this is a good and clean read! And I also think
this was long overdue.

I'd like to make an argument for using namespaces though. You write:

<quote>
Avoid using namespaces.  HotSpot code normally uses "all static"
classes rather than namespaces for grouping.  An "all static" class is
not instantiable, has only static members, and is normally derived
(possibly indirectly) from the helper class `AllStatic`.

Benefits of using such classes include:

* Provides access control for members, which is unavailable with
namespaces.

* Avoids ADL (Argument Dependent Lookup).

* Closed for additional members.  Namespaces allow names to be added in
multiple contexts, making it harder to see the complete API.

Namespaces should be used only in cases where one of those "benefits"
is actually a hindrance.
</quote>

I think "avoid" is a bit harsh; I think we should use them more. Often they
would be a cleaner solution than using static class methods. The points 1
and 3 you list are exactly the reason why I sometimes prefer them.

I do not like access control via C++ public/private membership as a way to
define subsystem APIs, because it does not easily allow to split an API in
public and internal parts across multiple headers.

Within a sub system, I'd like to keep public headers - those to be consumed
by code outside my subsystem - clean and the interfaces in them narrow.
They should pull as little additional headers as possible.

But AllStatic forces me to expose implementation details in headers. So
implementation details creep into headers which should be clean of them.
This can be seen in the hotspot in many places, e.g. in os_linux.hpp: of
the 370 lines of code in the os::Linux class, 150 are private or protected
members which really have no place in an outside interface (even worse in
this case, they leak into the "os" API too).

Exposing these details makes an interface convoluted. It is difficult to
see the real external interface with all the private chaff sprinkled in,
which often interleaves the public APIs. In that sense,  "making it harder
to see the complete API" as you put it is actually an advantage, since it
allows me to only see the public API. And the lack of C++ access control
forces the developer to split off private APIs from public ones.

It pulls in unnecessary headers which are needed for the private
implementation. This quickly leads to a "big ball of includes". That to me
is a real gripe.

It also tempts programmers to "just make it quickly public", to expose a
private API to fix something where it may have been nicer to keep the
interface narrow and find a different solution. So I think AllStatic is not
conducive to clean interface design.

With namespaces, OTOH, one can separate the public API from private APIs by
moving them into different headers. So, I could have two headers, one
"subsystem.hpp" for external consumption and one "subsystem_internals.hpp"
for the subsystem itself. The former can be very brief, only includes
whatever is needed for the interface prototypes. The latter can include
whatever it wants since it is only ever seen by the subsystem
implementation itself.

As for class os and "os::<platform>" and "os::<cpu>" in particular, here we
include the nested "namespaces" directly into the middle of class os. I
think these are ugly crutches and can cause confusion. For instance, one
always has to remember that os_xxx.hpp files are not "real" headers, e.g.
should not include any other headers. os and os::<platform> etc could be
defined a lot cleaner with nested namespaces.

Therefore I suggest we allow namespaces. Maybe agree on a convention of how
to use them.

THanks, Thomas