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

[Thomas Schatzl]

On Sat, 21 May 2022 11:05:11 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> 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.
>
>> Also note that in a (non-definitional) declaration, a top-level CV qualifier
> is ignored. So a parameter of type T* const is the same as a parameter of
> type T*. The const only has impact in a definition, when it declares the
> variable (as opposed to its value) as constant.
> 
> That may well be (and actually I tried it out and you are right - that has been new to me), I would prefer to keep the declaration and the definition exactly the same, even if superfluous.
> 
> E.g. we could write
> 
>   class X {
>     void square(char* x) const;
>   };
>   void X::square(char* const x) const {
>     ...
>   }
> 
> and this compiles. I would strongly prefer to not do this and keep declaration and definition exactly the same.

>>    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.

It would not hurt to have `const T* const pb` for that case either, because the method isn't supposed to modify the referenced contents either - however the main point of the author in that case (and what I'm trying to convey here, in at least many of the other cases as well) that the pointer value itself should be constant, and it would be nice to have that reflected.

E.g. the author saw:


void method(const oop b);

and then added, because it was needed:

void method(const HeapWord* b);

as a variant.

Note that maybe I'm wrong here, and the original author intended `const T* const something` for all these cases; but I'd argue there is very little const`ing to that degree in Hotspot code. There is of course also room for improvement here.

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

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