RFR: 8287089: G1: Change const <type>* name to <type>* const name in arguments

[Thomas Schatzl]

On Sat, 21 May 2022 03:11:19 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Hi all,
>> 
>>   can I have reviews for this change that fixes as few `const` modifiers, making them more consistent; mostly changing `const <type>* something` to `<type>* const something` as in many cases the pointer value should be constant, not the thing that is pointed to.
>> 
>> I also considered changing `const <typedef> something` where `typedef` is a pointer type (e.g. `oop` is `oopDesc*`), but `const oop something` is equal to `oop const something` (it only concerns `oop` in our code) to minimize the changes. If you think we should change these argument declarations to `oop const something` to be similar to other code, I can do that.
>> 
>> This is a preparatory change for [JDK-8210708](https://bugs.openjdk.java.net/browse/JDK-8210708) where some of the affected method get another parameter, and it looks weird to have e.g. `block_start(const HeapWord* addr, HeapWord* const pb)`, i.e. two parameters where although both are meant to be constant pointers, they have different `const`s.
>> 
>> Testing: local compilation, GHA
>> 
>> Thanks,
>>   Thomas
>
> src/hotspot/share/gc/g1/g1BlockOffsetTable.hpp line 124:
> 
>> 122:   void set_remainder_to_point_to_start_incl(size_t start, size_t end);
>> 123: 
>> 124:   inline size_t block_size(HeapWord* const p) const;
> 
> This looks wrong.  I don't think this operation modifies (nor should it) the contents of p, so the original declaration is correct.  The two are not equivalent (`const T*` is "pointer to constant T", while `T* const` is "constant pointer to T").  Also, a top-level `const` qualifier in a (non-definitional) declaration is ignored.  Similarly for the other changes in this file.

First, I understand your concerns, and I'm open to retract this change.

`const T*` is correct and I understand what it means. However I think many of these `const T*` were not what has been intended. The code does not dereference T at all, but does pointer manipulation throughout. So I think the intended use for the consts here (and in other places) has been to guard against manipulation of the passed arguments (and in most cases, where there is a mix of const'ed and non-consted pointers in the argument list, the non-consted are indeed modified and used as local variables), hence the change.

Another option is to change stuff to `const T* const something` to cover both cases (for this one).

>  I don't know what "pb" is in that signature, but the const qualifier for it isn't useful.

Similar to many other parameters here, pb is going to be (random) pointer into the heap region where the its value should not change in the method.

Again, I'm open to retract this change.

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

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