On Sat, 24 Oct 2020 22:22:56 GMT, Peter Levart <plev...@openjdk.org> wrote:

>> Reference instances should not be leaked and so I don't see very common that 
>> caller of `Reference::get` does not know the referent's type.   It also 
>> depends on the `refersTo` check against `null` vs an object.  Any known use 
>> case would be helpful if any (some existing code that wants to call 
>> `refersTo` to compare a `Reference` of raw type with an object of unknown 
>> type).
>> 
>> FWIW, when converting a few use of `Reference::get` to `refersTo` in JDK, 
>> there is only one case (`equals(Object o)` method that needs the cast.
>> 
>> http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8188055/jdk-use-refersTo/index.html
>
> @mlchung I don't have many known use cases, but how about 
> WeakHashMap.containsKey(Object key) for example? Currently 
> `WeakHashMap.Entry<K, V> extends  WeakReference<Object>` but it would be more 
> type safe if it extended `WeakReference<K>`. In that case an 
> `entry.refersTo(key)` would not compile...
> What I'm trying to say is that even if `Reference` instances are not 
> "leaked", you can get an untyped object reference from outside and you may 
> want to identity-compare it with the Reference's referent.

Some thoughts regarding the parameter type of refersTo. Summary: I think 
`refersTo(T)` is fine and that we don't want to change it to `refersTo(Object)`.

I don't think we have a migration issue similar to generifying collections, 
where there was a possibility of changing `contains(Object)` to `contains(E)`. 
If that had been done, it would have been a source compatibility issue, because 
changing the signature of the method potentially affects existing code that 
calls the method. That doesn't apply here because we're adding a new method.

The question now falls to whether it's preferable to have more convenience with 
`refersTo(Object)` or more type-safety with `refersTo(T)`. With the generic 
collections issue, the migration issue probably drove the decision to keep 
`contains(Object)`, but this has resulted in a continual set of complaints 
about the lack of an error when code passes an instance of the "wrong" type. I 
think that kind of error is likely to occur with `refersTo`. Since we don't 
have a source compatibility issue here, we can choose the safer API and avoid 
this kind of problem entirely.

The safer API does raise the possibility of having to add inconvenient 
unchecked casts and local variables in certain places, but I think Mandy's 
comment about the code already having a reference of the "right" type is 
correct. Her prototype webrev linked above shows that having to add unchecked 
casts is fairly infrequent.

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

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

Reply via email to