RFR: 8303621: BitMap::iterate should support lambdas and other function objects [v2]

Kim Barrett kbarrett at openjdk.org
Tue Mar 7 12:08:36 UTC 2023


On Tue, 7 Mar 2023 09:58:24 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> src/hotspot/share/utilities/bitMap.inline.hpp line 273:
>> 
>>> 271:     }
>>> 272:   }
>>> 273: }
>> 
>> IMHO, this is an eyesore that I prefer if it could be hidden away a bit. Could we create an `invoke` function that that calls this, and change the code to:
>> 
>> template <typename Function>
>> inline bool BitMap::iterate(Function function, idx_t beg, idx_t end) const {
>>   for (idx_t index = beg; true; ++index) {
>>     index = get_next_one_offset(index, end);
>>     if (index >= end) {
>>       return true;
>>     } else if (!invoke(function, index)) {
>>       return false;
>>     }
>>   }
>> }
>> 
>> 
>> And also add a comment why we need IterateInvoker?
>
> I added a local function object so that template clutter wasn't all right in the middle of the function.
> Making it a member function instead seemed to just push around the deck chairs.
> Added comment, and also fixed a lurking, probably never to be hit, bug that we were stopping early if
> ReturnType::operator! returned true, rather than if the converted to bool result was false.

Here's an alternative, using member function templates instead of the
partially specialized `IterateInvoker` function object. This doesn't seem
obviously better to me. Other ideas I looked at involved messy SFINAE, some
with trailing return types (a not yet approved for HotSpot C++11 feature).


template<typename Function, typename Dispatch>
static bool iterate_invoke_aux(Function function, idx_t index, Dispatch*) {
  return function(index);
}

template<typename Function>
static bool iterate_invoke_aux(Function function, idx_t index, void*) {
  function(index);
  return true;
}

template<typename Function>
static bool iterate_invoke(Function function, idx_t index) {
  using ReturnType = decltype(function(index));
  return iterate_invoke_aux(function, index, static_cast<ReturnType*>(nullptr));
}

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

PR: https://git.openjdk.org/jdk/pull/12876


More information about the hotspot-dev mailing list