On Tue, 15 Nov 2022 15:09:02 GMT, Per Minborg <pminb...@openjdk.org> wrote:
>> Maurizio Cimadamore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add `since` tag in Module/ModuleLayer preview methods
>
> src/java.base/share/classes/java/lang/foreign/Arena.java line 89:
>
>> 87:
>> 88: /**
>> 89: * Returns a native memory segment with the given size (in bytes)
>> and alignment constraint (in bytes).
>
> It is noted that the current documentation does not require a **new** native
> memory segment to be returned. Would it not be better with:
>
> Creates a new native memory segment ...
>
> The new shared segment might share actual backing memory though.
My feeling is that being overly precise over identity might backfire. It is not
important whether the segment is a new instance or not. But there is, perhaps,
another invariant that is more semantically relevant: e.g. the returned
segments (whether new or not, we don't care) should be backed by "disjoint"
regions of memory. That is, if the method returns a segment with address `0`
and size `100`, calling the method again cannot return a segment whose address
is `50` and size is `100`. In principle, the segment allocator interface allows
for this (see `SegmentAllocator::prefixAllocator`) - but for an arena, a
behavior such as this would be indesirable, IMHO.
> src/java.base/share/classes/java/lang/foreign/Arena.java line 119:
>
>> 117:
>> 118: /**
>> 119: * {@return the arena scope}
>
> Add a period ('.') after the closing curly bracket. This is a general comment.
I don't think we did this consistently in other places, I'd prefer to leave as
is.
> src/java.base/share/classes/java/lang/foreign/Arena.java line 136:
>
>> 134:
>> 135: /**
>> 136: * {@return {@code true} if the provided thread can close this
>> arena}
>
> I think this is equivalent and simpler:
>
> {@return if the provided thread can close this arena}.
>
> But I know there are many examples of {@code true} in the JDK.
I'll leave as is - we can deal with this cosmetic javadoc issues at a later
point.
> src/java.base/share/classes/java/lang/foreign/GroupLayout.java line 46:
>
>> 44:
>> 45: /**
>> 46: * Returns the member layouts associated with this group.
>
> We may use {@return the member layouts associated with this group}.
Same - I'll leave these tweaks for later.
> src/java.base/share/classes/java/lang/foreign/Linker.java line 264:
>
>> 262:
>> 263: /**
>> 264: * Returns a symbol lookup for symbols in a set of commonly used
>> libraries.
>
> Use {@return ...}
Same - I'll leave these tweaks for later.
-------------
PR: https://git.openjdk.org/jdk/pull/10872
