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

[Thomas Schatzl]

On Sat, 21 May 2022 10:53:46 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

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

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

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

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