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