RFR: 8189088: Add intrusive doubly-linked list utility [v8]

Kim Barrett kbarrett at openjdk.org
Wed Oct 18 01:10:51 UTC 2023


On Tue, 17 Oct 2023 20:16:41 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Kim Barrett has updated the pull request incrementally with four additional commits since the last revision:
>> 
>>  - remove unnecessary code markers in comments
>>  - use override for virtual test support functions
>>  - remove support for allocation base classes
>>  - remove IntrusiveList::operator[]
>
> src/hotspot/share/utilities/intrusiveList.hpp line 812:
> 
>> 810:   static constexpr bool can_splice_from() {
>> 811:     return Conjunction<Impl::IsListType<Other>,
>> 812:                        std::is_convertible<typename Other::iterator, iterator>>::value;
> 
> Here and in can_swap we're using `Conjunction` rather than simple `&&`
> expressions of the values. The intent is to delay the iterator check until
> we've verified `Other` is actually a List type. But that isn't working.
> Instead, if `Other` isn't a List then we get a syntax error on
> `Other::iterator`, which is not at all helpful as error reporting goes. To fix
> this we need to add a level of indirection so the code containing
> `Other::iterator` doesn't get instantiated until the IsListType check has
> passed.

Fixed.

> src/hotspot/share/utilities/intrusiveList.hpp line 1334:
> 
>> 1332:    * transferred).
>> 1333:    */
>> 1334:   template<typename FromList, ENABLE_IF(Impl::IsListType<FromList>::value)>
> 
> Why is this not using `can_splice<FromList>()`?  In some earlier development version doing something like that
> produced much worse compiler error messages than doing the more limited test and leaving the compatibility
> checking to the inner call to `splice` with range arguments.  But I think that shouldn't be true now, and this
> should be changed to use `can_splice`.  But look at the compiler error messages before making that change.

Fixed.  Changing it even made the error message better.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15896#discussion_r1362973509
PR Review Comment: https://git.openjdk.org/jdk/pull/15896#discussion_r1362973477


More information about the hotspot-dev mailing list