On Thu, 11 Nov 2021 14:50:43 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:

> I ran bin/blessed-modifier-order.sh on source owned by Project Panama. This 
> scripts verifies that modifiers are in the "blessed" order, and fixes it 
> otherwise. I have manually checked the changes made by the script to make 
> sure they are sound.
> 
> In this case, while the script did into the "correct" thing, it turns out 
> that the method signatures in 
> `src/jdk.incubator.vector/share/classes/jdk/incubator/vector` has some room 
> for improvement... The files contains method headers which look like this:
> 
> 
>         final @Override
>         @ForceInline
>         long longToElementBits(...
> 
>         @ForceInline
>         static long toIntegralChecked(...
> 
>         @ForceInline
>         @Override final
>         ByteVector dummyVector(...
> 
> 
> My personal opinion is that these should have been written like this:
> 
> 
>         @Override
>         @ForceInline
>         final long longToElementBits(...
> 
>         @ForceInline
>         static long toIntegralChecked(...
> 
>         @ForceInline
>         @Override
>         final ByteVector dummyVector(...
> 
> 
> or possibly
> 
> 
> 
>         @Override @ForceInline
>         final long longToElementBits(...
> 
>         @ForceInline
>         static long toIntegralChecked(...
> 
>         @ForceInline @Override
>         final ByteVector dummyVector(...
> 
> 
> If you want me to make that change as well as part of the fix, let me know.
> 
> Furthermore, I don't know how much the code in mainline differs from the code 
> in the Panama branches. If the discrepancy is large, you might want to run 
> `bash bin/blessed-modifier-order.sh src/jdk.incubator.vector` and  `bash 
> bin/blessed-modifier-order.sh src/jdk.incubator.foreign` in those branches.

On a  second look, changes are relatively contained. I'm ok with integrating 
this ahead of the Panama/foreign refresh. I found a possible issue in the 
vector API, with `public` being of the same line as the annotation, which looks 
odd (as you have noticed).

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteVector.java 
line 3908:

> 3906: 
> 3907:         @ForceInline
> 3908:         @Override public

`public` should probably go on a different line? (same as for the ones that 
follow)

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

PR: https://git.openjdk.java.net/jdk/pull/6355

Reply via email to