RFR: 8271525: ResourceHashtableBase::iterate() should not declared as const

[Coleen Phillimore]

On Fri, 30 Jul 2021 09:26:00 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> `ResourceHashtableBase::iterate()` is declared `const`, but it can actually change the contents of the table. The same is true for `ResourceHashtableBase::get()`, which returns a non-`const` pointer to the value, allowing the caller to modify it.
>> 
>> We should declare these two functions as non-`const`. This will also remove a lot of ugly `const_cast<>` code.
>> 
>> The `iterate()` API is tightened such that the `do_entry()` function can modify the `value` but not the `key`.
>
> I think this is not an improvement, sorry. Now, a number of methods became non-const, including `iterate`, and that non-constness propagates. `iterate` itself does not change the table, and it can be reasonably called from const contexts. There are actually a number of callers which arguably should be const but aren't (e.g. `ClassLoaderStatsClosure::print()`.
> 
> I think a better approach would be to have both const and non-const versions, e.g. for `bucket_at()`:
> 
> 
>   Node** bucket_at(unsigned index) ...
>   const Node* const * bucket_at(unsigned index) const ...
> 
> 
> This way, the compiler chooses the const version in const code, giving you the benefit of a read-only V*, and in non-const code you get access to the innards of V*. No awkward const casts needed.
> 
> Above code makes Coleen's new unlink() from https://github.com/openjdk/jdk/pull/4938 compile in non-const form.
> 
> Note how `ResourceHashTable::lookup_node()` already follows this pattern. You deleted the non-const version, but it made sense.
> 
> Same with `::get`:
> 
> 
> const V* get(K const& key) const;
> V* get(K const& key);
> 
> 
> Making the Key references const is fine though.
> 
> ..Thomas

@tstuefe I was not paying attention when C++ added overloading based on constness of return type so that's an interesting change, but also, does this get rid of the monstrosity:

    return const_cast<Node const**>(
        const_cast<ResourceHashtableBase*>(this)->lookup_node(hash, key));
?

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

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