[vector] RFR -- removing Shape as type parameter

[Vladimir Ivanov]

I like this change. I had a mixed experience with API when Shape was 
involved: it worked well when concrete Shapes were used, but fell apart 
when I tried to code in shape-agnostic manner forcing me either to 
fallback to erased types or tricky usages of type parameters.

I have only one comment: on API level S_Max_BIT overlaps with 
Vector.preferredSpecies(), so it was agreed to not to expose it on API 
level when [Int|...]MaxVector's were incorporated. With your change it 
becomes part of the API:

+public enum Shape {
+    S_64_BIT(64),
+    S_128_BIT(128),
+    S_256_BIT(256),
+    S_512_BIT(512),
+    S_Max_BIT(Unsafe.getUnsafe().getMaxVectorSize(byte.class) * 8);

-    static final SMaxBit S_Max_BIT = new SMaxBit();
-    static final class SMaxBit extends Vector.Shape {

I don't have a strong preference here: preferredSpecies() looks more 
flexible, but I see that S_Max_BIT can be more convenient to use. My 
point is: only one should stay.

I don't see it as a pressing issue. Just want to keep it on the table, 
so it will be addressed later as part of followup API refactorings.

Best regards,
Vladimir Ivanov

On 28/11/2018 08:54, Brian Goetz wrote:
> The below patch (tests pass) removes the Shape type argument from Vector 
> and subtypes.
> 
>      http://cr.openjdk.java.net/~briangoetz/panama/noShape.patch
> 
> The use of Shape as a generic type parameter is clever, in that it does 
> a good job of providing compile-time type safety (preventing us from 
> mixing vectors of the wrong species), but I think we might be pushing it 
> too hard here, and I think it can be profitably removed. It is rare to 
> be mixing vectors who have the same element type but different shapes; 
> when we do, these can (and must) be caught by runtime checks (regardless 
> of whether we’re using static type checking to minimize the risk of 
> mixing vectors of incompatible sizes, we still have to check that things 
> match in the implementations (as the user could be using erased types.) 
> The Shape parameter bubbles out into the user code, for relatively 
> little benefit.  I don't think it carries its weight.
> 
> Removing the Shape parameter also lets us undo another distortion, which 
> is making the Shape constants into separate classes.  Now they can be a 
> regular enum.  (Similarly, this also clears the way for some downstream 
> improvements to the API.)
> 
> (The patch is large as it includes the changes to the generated files 
> and tests, but all changes were made to the underlying templates. Here's 
> a smaller patch, which only contains changes to the source templates and 
> non-templated source files, in case that's easier to review.)
> 
> http://cr.openjdk.java.net/~briangoetz/panama/noShapeSourceTemplatesOnly.patch 
> 
> 
>