RFR: JDK-8312395: Improve assertions in growableArray [v2]
David Holmes
dholmes at openjdk.org
Thu Jul 20 10:32:46 UTC 2023
On Thu, 20 Jul 2023 08:47:49 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:
>> There are a number of assertions in growableArray , for example to check for a valid index to access/remove elements.
>> Those assertions can be improved, e.g. by showing the bad index value used in case of failure.
>>
>> Example for an assertion in the 'at(index i)' access method
>> old assertion looks like
>> assert(0 <= i && i < _len) failed: illegal index
>>
>> new assertion looks like
>> assert(0 <= i && i < _len) failed: illegal index -559030609, 1 accessible elements
>
> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
>
> adjust some output and some asserts
One further suggestion but approving anyway.
I think the use of `this->xxx` is just personal style. I find it unnecessary especially given the other uses in this file.
Thanks.
src/hotspot/share/utilities/growableArray.hpp line 263:
> 261: void remove_range(int start, int end) {
> 262: assert(0 <= start, "illegal start index %d", start);
> 263: assert(start < end && end <= _len, "erase called with invalid range %d, %d", start, end);
I think you'd want to see `_len` here too - suggestion:
"erase called with invalid range (%d, %d) for length %d", start, end, _len
-------------
Marked as reviewed by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14946#pullrequestreview-1538947826
PR Review Comment: https://git.openjdk.org/jdk/pull/14946#discussion_r1269263868
More information about the hotspot-dev
mailing list