RFR: 8188055: (ref) Add Reference::refersTo predicate [v5]

[David Holmes]

Hi Kim,

On 19/10/2020 9:29 pm, Kim Barrett wrote:
>> On Oct 18, 2020, at 5:36 PM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> On 17/10/2020 1:23 am, Kim Barrett wrote:
>>>> src/java.base/share/classes/java/lang/ref/Reference.java line 348:
>>>>
>>>>> 346:      * Tests if this reference object refers to {@code obj}.  If {@code obj} is
>>>>> 347:      * {@code null}, this method returns {@code true} if this reference object
>>>>> 348:      * refers to {@code null} or has been cleared.
>>>>
>>>> Is there actually a distinction between clearing and referring to null?
>>> A (not very useful) Reference can be created with a null referent.
>>
>> Yes but you cannot distinguish between the two cases yet this wording is trying to make that distinction. My concern is interaction with the wording here:
>>
>>   * @return  {@code true} if and only if {@code obj} is the referent
>>   *          of this reference object
>>
>> As written it implies to me that given:
>>
>> Object o = new Object();
>> PhantomReference r = new PhantomReference(o);
>> r.clear();
>> assert(r.refersTo(null)); // pass or fail?
>>
>> then the assert should fail because "null" was not the referent, as r has been cleared - which are two distinct cases according to the initial java doc. The initial doc would be clearer if it stated words to the effect of:
>>
>> "returns {@code true} if this reference object refers to {@code null} (either because it was constructed that way, or it has been cleared)."
>>
>> or else the @return doc needs to account for the null case more explicitly.
> 
> I don't see how you arrive at the assert failing. The referent is what get
> would return, by definition. And get returns null for a cleared reference.

That's the crux of it: what exactly is meant by "the referent"? Does it 
mean the original object that was used as the referent, or does it mean 
the current value of the "referent" field inside the Reference (as get 
might return)?

> The wording in the initial javadoc is the result of offline requests from
> Mandy from before the first public version.  Before that I had just
> 
> * Tests if {@code obj} is the referent of this reference object.
> 
> My recollection (the discussion is lost - darn slack retention limits) is

I had some input in the early days too, but then missed the CSR and 
related discussion - apologies for that.

> that Mandy felt the reliance on the meaning of "referent" from get was too
> indirect, and that some additional discussion of the null case was needed
> for refersTo. Personally, I'd be happy with deleting that 2nd sentence, and
> even reverting to my original description, but Mandy (and others) might not
> agree.

I think we've ended up with the main doc and the @return being 
inconsistent in that regard. They should either both talk about 
"referent" or both be expressed in terms of "refers to" (the latter more 
clearly indicates we are talking about the value of a field, or what 
get() might return).

> 
> Maybe something like the following?
> 
> * Tests if {@code obj} is the referent of this reference object.
> * If {@code obj} is {@code null}, this method returns {@code true} if
> * this reference object was constructed with a {@code null} referent
> * or has been cleared.

That reads well to me and is more consistent with the @return wording. I 
personally prefer the "refers to" terminology, but either way is fine as 
long as the two parts are consistent.

>> Also why the "if and only if" in the @return doc versus a simple "if"? What is being emphasized by using "if and only if"?
> 
> "if and only if" is the same phrasing used by isEnqueued; I just followed
> that, perhaps overly slavishly.  It seems like that's pretty common usage
> elsewhere too.  But I don't have any objection to removing "and only if" if
> that's what the experts want.  (I'm not really a Java programmer.)

To me the "if and only if" is emphasizing something - it suggests that 
simply using "if" allows a possibility that might be surprising. In this 
case it suggested to me a distinction between being the original (null) 
referent, and just happening to be null due to clearing (in part because 
the main doc comment seemed to be making that distinction).

Specs are tricky and it can get quite pedantic and finicky. Someone, at 
some point will be given this API description and asked to write 
conformance tests, so it needs to be clear and unambiguous.

Hopefully Mandy will re-engage on this aspect.

Just to be clear, the current wording is not terrible, I'm just looking 
at potential improvement.

Thanks,
David
-----

>