Propose to use clang-format to enforce hotspot codestyle

[Volker Simonis]

Jesper Wilhelmsson <jesper.wilhelmsson at oracle.com> schrieb am Mi., 11. März
2020, 19:13:

> On 11 Mar 2020, at 13:06, Volker Simonis <volker.simonis at gmail.com> wrote:
>
> Liu, Xin <xxinliu at amazon.com> schrieb am Mi., 11. März 2020, 09:50:
>
> Hi,
>
> I just filed a JBS issue JDK-8240834 about hotspot codestyle(
> https://wiki.openjdk.java.net/display/HotSpot/StyleGuide). I found many
> inconsistent code in hotspot so I propose to use clang-format[1] to enforce
> it.
>
> I know you’re all very experienced developers, but sometimes wrong
> indentations or tailing whitespaces creep in.  it's hard to catch them in
> webrev.  Even though we can, new contributors still need to contact their
> sponsors for formatting changes.   I think it's a good idea to have lint
> thing done at first place.
>
> Clang is powerful enough to parse modern C/C++ code and it's very handy to
> format code. clang-format should be available on all supporting platforms.
> It's still okay even if it is absent on some platforms because nightly
> build and submit repo will cover unsupported platforms.
>
> I plan to have  a .clang-format and correcting styles in the 1st patch. We
> can bring auto clang-format check to hotspot's makefile in the second
> patch.  Does it sound like a plan?
>
>
> Have you already checked how big this first change will be? I'm afraid it
> will be huge because we have a lot of manually formatted code which doesn't
> confirm to any specific style (e.g. macros). We also have consistent but
> different code styles in various parts of HS. So changing all the sources
> to conform to a single style will necessarily be quite invasive and I'm not
> sure if we want that?
>
>
> In theory I support using a formatting tool to maintain a single code
> style throughout the hotspot code, but in practice this is extremely
> difficult. Volker brings up one point here, the situation today is bad -
> really bad - from a formatter's perspective. The size of the initial change
> will be huge. To review that requires people from all affected areas to
> look in detail on their code to make sure nothing is broken.  Andrew brings
> up a different point in his mail; parts of the code is manually formatted
> for a reason. No automatic formatter will be able to make sense of all code
> if it applies the same rules all over.
>
> I have looked into this problem for quite some time and I believe we have
> to take a very slow approach to this. Not only to be able to actually
> review the changes, but also to keep our developers around. First, any
> changes we make to clean up formatting must be automatically verified from
> that point forward to stop people from pushing new code that violates the
> rules we have managed to agree on. This requires a tool that can enable
> individual checks one by one. Secondly, the tool can't go in and modify
> people's code, it can only verify that it is correct and give a warning if
> there is a violation. This means that what we are looking for is a
> validator, not a formatter. There must also be the option to disable
> validation for parts of the code.
>
> I have not found any such tools out there, one that can accept the hotspot
> source code as is without any remarks if all checks are disabled, and then
> let you enable one check a time as we gradually clean up the code. The best
> I could find was uncrustify, but it still is a formatter, not a validator,
> and there are a few style rules that can't be disabled. I asked the
> developers of uncrustify if there was any chance of having the option for
> it to be a validator instead but that didn't seem to be in their interest.
>
> As I didn't find one I started to implement my own validator a few years
> ago. I do have a tool that swallows most of the hotspot code as is, but
> there are a few issues with macro usages. You see, that is another issue we
> have to deal with. It's not enough to parse C++. If we want to read and
> format the code we need to treat our macros as part of the language as
> well. Hotspot has a few macros that make for real challenges for a parser
> :-)
>

The default Emacs on Ubuntu 18.04 will just hang forever consuming 100% cpu
when opening globals.hpp with syntax highlighting enabled. :)

 I haven't yet included the option to disable verification for a block of
> code, coming to think of it, that could actually solve the most weird macro
> usages we have out there. I'll take another look at that at some point.
>
> Until we have a tool of this kind I'm afraid it will be extremely
> difficult to enforce any style rules in practice. But I do agree that this
> is a goal worth pursuing.
>
> Cheers,
> /Jesper
>
>
>
> Reference:
> [1] https://clang.llvm.org/docs/ClangFormat.html
> Thanks,
> --lx
>
>
>