RFR: 8332176: Refactor ClassListParser::parse() [v2]

[Ioi Lam]

On Tue, 14 May 2024 18:54:59 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - @calvinccheung and @matias9927 comments
>>  - Merge branch 'master' into 8332176-refactor-class-list-parser-parse
>>  - 8332176: Refactoring ClassListParser::parse()
>
> src/hotspot/share/cds/classListParser.cpp line 250:
> 
>> 248: }
>> 249: 
>> 250: void ClassListParser::split_tokens_by_whitespace(int offset, GrowableArray<const char*>* items) {
> 
> Unless I'm mistaken, `split_tokens_by_whitespace` is only called once by`parse_at_tags`. Do you anticipate that this will be called with multiple "item" lists in the future? If not I think you can leave `_indy_items` in this method as before.

I changed this function as its name is confusing. If we want to keep the existing code that operates on `_indy_items`, we have to rename it accordingly, as `split_tokens_by_whitespace` doesn't mention anything about  `_indy_items`. But that seems to be going the wrong direction, making it more difficult to reuse this function in the future, if such need arises.

I think it's a good idea to reduce coupling in the code. This function splits a line into multiple parts, so we shouldn't make it depend on `_indy_items`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19225#discussion_r1600898987