RFR: 8312502: Mass migrate HotSpot attributes to the correct location [v2]

[David Holmes]

On Sun, 23 Jul 2023 11:35:19 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Julian Waters has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 40 additional commits since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into patch-6
>>  - arguments.hpp
>>  - arguments.hpp
>>  - globalDefinitions_gcc.hpp
>>  - assembler_aarch64.hpp
>>  - macroAssembler_aarch64.cpp
>>  - vmError.cpp
>>  - vmError.cpp
>>  - macroAssembler_aarch64.cpp
>>  - assembler_aarch64.hpp
>>  - ... and 30 more: https://git.openjdk.org/jdk/compare/f058acfd...afff56f2
>
> src/hotspot/share/c1/c1_CFGPrinter.hpp line 66:
> 
>> 64:   void dec_indent();
>> 65:   ATTRIBUTE_PRINTF(2, 3)
>> 66:   void print(const char* format, ...);
> 
> This is an example where rearranging the attributes is out of character with usual practice.
> And I think it makes it harder to read.

I agree with Kim, I do not like this style of using a new line for the attribute. I also prefer to see these attributes in their original location where I can generally ignore them while reading the code.

> src/hotspot/share/utilities/xmlstream.hpp line 149:
> 
>> 147:   void          text(const char* format, ...);
>> 148:   ATTRIBUTE_PRINTF(2, 0)
>> 149:   void       va_text(const char* format, va_list ap) {
> 
> This file is a particularly bad (to me) example of what happens without whitespace between a declaration
> and the attributes for the next declaration.  I find this really hard to parse.  And the extra whitespace following
> return types makes it even worse for me.

+1

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14969#discussion_r1271561053
PR Review Comment: https://git.openjdk.org/jdk/pull/14969#discussion_r1271561299