RFR(s): 8203670: unmodifiable List iterator() implementations should not be ListIterators

[Ivan Gerasimov]

Okay.  Thanks for explaining!

With kind regards,

Ivan


On 6/26/18 10:51 AM, Stuart Marks wrote:
> On 6/26/18 9:57 AM, Ivan Gerasimov wrote:
>> On the first place, I'd like to understand why it is bad, if 
>> List.of().iterator() in fact returns ListIterator?
>> What kind of problems it may cause?
>
> Basically it leaks information. Somebody might get an iterator, 
> partially advance it, and hand it off to another piece of code, 
> assuming that it can only be advanced. But if that instance can be 
> downcast and used as a ListIterator, that assumption would be 
> violated. I admit it's a narrow case, but it's nonetheless a possibility.
>
>
>> Out of curiosity.  What if someone does something like
>>
>> if (it instanceof ListIterator) {
>>     // optimized for bidirectional access
>> } else {
>>     // slower algorithm
>> }
>>
>> previous(), nextIndex() and previousIndex() methods are not declared 
>> to be optional, so is it appropriate to throw 
>> UnsupportedOperationException from them?
>>
>> Someone may assume that if an object can be cast to ListIterator 
>> interface, non-optional methods should not throw UOE.
>
> I think this usage is out of bounds for the List API. The List 
> specification is:
>
>     Iterator<E> iterator();
>     ListIterator<E> listIterator();
>     ListIterator<E> listIterator(int);
>
> That is, you get an Iterator from the iterator() method, and you get a 
> ListIterator from the listIterator() methods. This implementation 
> fulfils all the contracts for the interfaces declared as the return 
> types. In particular, if you ask for a ListIterator, you get a 
> ListIterator implementation that implements all the methods.
>
> However, if you call iterator() and get an Iterator, and do an 
> 'instanceof' and downcast, then all bets are off. It's reasonable to 
> rely on the thing returned by iterator() having any behaviors other 
> than those specified by Iterator.
>
>
> Peter Levart wrote:
>
>> Also, performance would be better if it was a separate class. It 
>> could be a superclass of ListItr so that common logic is not duplicated.
>
> This isn't at all obvious. Certainly it's not obvious enough to embark 
> on a refactoring without doing some benchmarking.
>
> However, I want to fix this bug in JDK 11. If somebody wants to do 
> some benchmarking, they're welcome to do so, but I don't want to 
> consider this as part of this changeset.
>
>> @Stable annotation on private final boolean isListIterator does not 
>> help here unless the returned Iterator object is assigned to a static 
>> final field and used from it. Hardly a common use case for 
>> Iterator(s) I guess.
>
> The exact effect of @Stable is Hotspot-specific and changes over time. 
> Here we're using it as a declaration that this final field really can 
> be treated as final, and that it won't be modified via reflection or a 
> varhandle, thus enabling VM more aggressive optimizations.
>
> s'marks
>
>

-- 
With kind regards,
Ivan Gerasimov