LGTM.
The illustrative example in asUnsigned is nice ( I should have thought of
similar, D’oh! )
-Chris.
> On 3 Jun 2020, at 15:25, Maurizio Cimadamore <maurizio.cimadam...@oracle.com>
> wrote:
>
> Please review this delta patch; I realized that the previously submitted
> patch had few issues:
>
> * copyright header on TestAdaptVarHandle benchmark was wrong
> * stream test for segment spliterator was not updated to include changes to
> copyFrom API
> * I've also incorporated a small javadoc tweak to MemoryHandles::asUnsigned
> that was suggested in the CSR process
>
> Here's the patch (if a webrev if preferred, I can also generate that):
>
> diff -r 6b40a32ede25
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryHandles.java
> ---
> a/src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryHandles.java
> Wed Jun 03 13:57:18 2020 +0100
> +++
> b/src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryHandles.java
> Wed Jun 03 15:22:03 2020 +0100
> @@ -357,9 +357,14 @@
> * primitive data types as if they were a wider signed primitive type.
> For
> * example, it is often convenient to model an <i>unsigned short</i> as a
> * Java {@code int} to avoid dealing with negative values, which would be
> - * the case if modeled as a Java {@code short}. The returned var handle
> - * converts to and from wider primitive types, to a more narrow possibly
> - * unsigned primitive type.
> + * the case if modeled as a Java {@code short}. This is illustrated in
> the following example:
> + * <blockquote><pre>{@code
> + MemorySegment segment = MemorySegment.allocateNative(2);
> + VarHandle SHORT_VH = MemoryLayouts.JAVA_SHORT.varHandle(short.class);
> + VarHandle INT_VH = MemoryHandles.asUnsigned(SHORT_VH, int.class);
> + SHORT_VH.set(segment.baseAddress(), (short)-1);
> + INT_VH.get(segment.baseAddress()); // returns 65535
> + * }</pre></blockquote>
> * <p>
> * When calling e.g. {@link VarHandle#set(Object...)} on the resulting
> var
> * handle, the incoming value (of type {@code adaptedType}) is converted
> by a
> diff -r 6b40a32ede25
> test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/SegmentTestDataProvider.java
> ---
> a/test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/SegmentTestDataProvider.java
> Wed Jun 03 13:57:18 2020 +0100
> +++
> b/test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/SegmentTestDataProvider.java
> Wed Jun 03 15:22:03 2020 +0100
> @@ -23,7 +23,6 @@
>
> package org.openjdk.tests.java.util.stream;
>
> -import jdk.incubator.foreign.MemoryAddress;
> import jdk.incubator.foreign.MemoryLayout;
> import jdk.incubator.foreign.MemoryLayouts;
> import jdk.incubator.foreign.MemorySegment;
> @@ -116,9 +115,9 @@
>
> static Consumer<MemorySegment> segmentCopier(Consumer<MemorySegment>
> input) {
> return segment -> {
> - MemorySegment copy = MemorySegment.ofArray(new
> byte[(int)segment.byteSize()]);
> - MemoryAddress.copy(segment.baseAddress(), copy.baseAddress(),
> segment.byteSize());
> - input.accept(copy);
> + MemorySegment dest = MemorySegment.ofArray(new
> byte[(int)segment.byteSize()]);
> + dest.copyFrom(segment);
> + input.accept(dest);
> };
> }
>
> diff -r 6b40a32ede25
> test/micro/org/openjdk/bench/jdk/incubator/foreign/TestAdaptVarHandles.java
> ---
> a/test/micro/org/openjdk/bench/jdk/incubator/foreign/TestAdaptVarHandles.java
> Wed Jun 03 13:57:18 2020 +0100
> +++
> b/test/micro/org/openjdk/bench/jdk/incubator/foreign/TestAdaptVarHandles.java
> Wed Jun 03 15:22:03 2020 +0100
> @@ -1,25 +1,27 @@
> /*
> - * Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved.
> - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
> + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> *
> - * This code is free software; you can redistribute it and/or modify it
> - * under the terms of the GNU General Public License version 2 only, as
> - * published by the Free Software Foundation.
> + * This code is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 only, as
> + * published by the Free Software Foundation.
> *
> - * This code is distributed in the hope that it will be useful, but WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> - * version 2 for more details (a copy is included in the LICENSE file that
> - * accompanied this code).
> + * This code is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * version 2 for more details (a copy is included in the LICENSE file that
> + * accompanied this code).
> *
> - * You should have received a copy of the GNU General Public License version
>
> Maurizio
>
> On 28/05/2020 22:25, Maurizio Cimadamore wrote:
>> Hi,
>> this followup change includes a number of tweaks that have been added to the
>> memory access API while we were in the process of integrating it. Most of
>> them have been contributed by Chris (thanks!), and are all listed in great
>> details in the CSR below:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8246096
>>
>> Implementation-wise it should be all relatively straightforward - apart for
>> the bit of surgery on lambda forms which was required to add a new kind of
>> lambda forms to 'collect' return values. For now this method handle
>> adaptation is package private, and can only be used by
>> MemoryHandles::filterValues - but, if people find it useful, we might
>> consider adding it to the MethodHandle standard combinator toolbox.
>>
>> Cheers
>> Maurizio
>>
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~mcimadamore/8246095_v1/webrev/
>>
>> Javadoc:
>>
>> http://cr.openjdk.java.net/~mcimadamore/8246095_v1/javadoc/jdk/incubator/foreign/package-summary.html
>>
>>
>> Specdiff:
>>
>> http://cr.openjdk.java.net/~mcimadamore/8246095_v1/specdiff/overview-summary.html
>>
>>
