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

Kim Barrett kbarrett at openjdk.java.net
Mon Oct 12 16:00:15 UTC 2020


On Mon, 12 Oct 2020 07:43:19 GMT, Per Liden <pliden at openjdk.org> wrote:

>> Kim Barrett has updated the pull request incrementally with five additional commits since the last revision:
>> 
>>  - improve refersTo0 descriptions
>>  - basic functional test
>>  - change referent access
>>  - expand test
>>  - remove CMS comment
>
> test/jdk/java/lang/ref/ReferenceRefersTo.java line 58:
> 
>> 56:             fail(kind + " doesn't refer to expected value");
>> 57:         } else if (ref.refersTo(unexpectedValue)) {
>> 58:             fail(kind + " refers to unexpected value");
> 
> This part, and the use of obj3, below seems odd and unnecessary. If the Reference doesn't have the expected value
> there's no special reason to think it has obj3 as value (as opposed to something else). I don't think this check will
> help debugging if this test fails.

A possible failure mode is to implement the predicate incorrectly, perhaps flipping the sense of the predicate. This
may help notice such. It's distinct from the null test, since null is a special value that might be handled differently.

> test/jdk/java/lang/ref/ReferenceRefersTo.java line 42:
> 
>> 40:             this.value = value;
>> 41:         }
>> 42:     }
> 
> Introducing this class seems unnecessary, since its `value` is never used. So all instances of `TestObject` could just
> be `Object`.

I'd intended to use the value field when printing failure messages, but that became a little messy and probably not
worth the trouble in the face of possible null values. So yeah, just using Object is fine; I'll make that change.

> test/hotspot/jtreg/gc/TestReferenceRefersTo.java line 186:
> 
>> 184:
>> 185:             expectNotValue(testWeak2, testObject3, "testWeak2");
>> 186:             expectValue(testWeak3, testObject3, "testWeak3");
> 
> It looks to me like the `expectNotCleared()` and `expectNotValue()` methods can be removed. All expect-tests can be
> done with just `expectCleared()` and `expectValue()`.

I don't think so.  For example,
`expectNotCleared(testWeak1, "testWeak1")`
is not testing the same thing as
`expectValue(testWeak1, testObject1, "testWeak1")`
If the implementation is correct they will indeed always be consistent. But they could be different with an incorrect
implementation.

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

PR: https://git.openjdk.java.net/jdk/pull/498


More information about the core-libs-dev mailing list