RFR: 8331193: Return references when possible in GrowableArray [v7]

[Emanuel Peter]

On Wed, 22 May 2024 15:34:17 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Hi,
>> 
>> This PR introduces the possibility of using references more often when using GrowableArray, where as previously this was only possible when using the `at()` method. This lets us avoid copying and redundant method calls and makes the API more streamlined. After the patch, we can use `at_grow` just like `at` works. The same goes for `top`, `first`, and `last`.
>> 
>> 
>> Some example code:
>> ```c++
>> // Before this patch this worked:
>> GrowableArray<int> arr(8,8,-1); // Pre-fill with 8 -1s
>> int& x = arr.at(7);
>> if (x == -1) {
>>   x = 2;
>> }
>> assert(arr.at(7) == 2, "this holds");
>> // but this was forbidden
>> int& x = arr.at_grow(9, -1); // Compilation error! at_grow returns E, not E&
>> // so we had to do
>> int x = arr.at_grow(9, -1);
>> if (x == -1) {
>>   arr.at_put(9, 2);
>> }
>> 
>> 
>> Thanks.
>
> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add test

Changes requested by epeter (Reviewer).

test/hotspot/gtest/utilities/test_growableArray.cpp line 669:

> 667: TEST(GrowableArrayCHeap, ReturningReferencesWorksAsExpected) {
> 668:   GrowableArrayCHeap<int, mtTest> arr(8, 8, -1); // Pre-fill with 8 -1s
> 669:   int& x = arr.at_grow(9, -1);

Suggestion:

  int& x = arr.at_grow(9, -1);
  EXPECT_EQ(-1, arr.at(9));
  EXPECT_EQ(-1, x);

test/hotspot/gtest/utilities/test_growableArray.cpp line 672:

> 670:   x = 2;
> 671:   EXPECT_EQ(2, arr.at(9));
> 672:   x = arr.top();

This is not using reference, right? I thought you can only use reference assignment when you declare the `x` variable. This here is using value, I think.

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

PR Review: https://git.openjdk.org/jdk/pull/18975#pullrequestreview-2072954331
PR Review Comment: https://git.openjdk.org/jdk/pull/18975#discussion_r1611061861
PR Review Comment: https://git.openjdk.org/jdk/pull/18975#discussion_r1611063611