On Wed, 31 Jan 2024 00:34:40 GMT, Paul Sandoz <psan...@openjdk.org> wrote:

>> test/jdk/jdk/incubator/vector/templates/X-LoadStoreTest.java.template line 
>> 271:
>> 
>>> 269:     @DontInline
>>> 270:     static $abstractvectortype$ fromArray($type$[] a, int i) {
>>> 271:         return ($abstractvectortype$) SPECIES.fromArray(a, i);
>> 
>> These new tests focus on the species method - but should we also test the 
>> statics factories in the record class, just in case a regression is 
>> introduced and the two implementation start diverging again?
>
> My expectation is the risk is small, but of course non-zero. These tests can 
> be expensive to run so i was trying balance the risk without increasing test 
> execution times.
> 
> I could strengthen the comment from:
> 
>         @ForceInline
>         @Override final
>         public ByteVector fromMemorySegment(MemorySegment ms, long offset, 
> ByteOrder bo) {
>             // User entry point:  Be careful with inputs.
>             return ByteVector
>                 .fromMemorySegment(this, ms, offset, bo);
>         }
> 
> 
> to:
> 
> 
>         @ForceInline
>         @Override final
>         public ByteVector fromMemorySegment(MemorySegment ms, long offset, 
> ByteOrder bo) {
>             // User entry point
>             // Defer only to the equivalent method on the vector class, using 
> the same inputs
>             return ByteVector
>                 .fromMemorySegment(this, ms, offset, bo);
>         }

Sounds good

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17621#discussion_r1474374111

Reply via email to