RFR: 8341696: C2: Non-fluid StringBuilder pattern bails out in OptoStringConcat [v4]

[Emanuel Peter]

On Wed, 8 Jan 2025 13:25:52 GMT, Theo Weidmann <tweidmann at openjdk.org> wrote:

>> src/hotspot/share/opto/stringopts.cpp line 414:
>> 
>>> 412: }
>>> 413: 
>>> 414: PhaseStringOpts::CheckAppendResult PhaseStringOpts::check_append_candidate(CallStaticJavaNode* cnode,
>> 
>> Using two verbs in succession is a little confusing. At least write `check_and_append_candidate`. Or maybe `append_candidate_if_<something>`.
>
> It's not supposed to be a verb. This methods checks a potential `append` call (a call that might be a call to StringBuilder::append), so it's an *append candidate*. Do you have any suggestions to clarify that?

Hmm... Ok. In my opinion a `check` method should be "pure", so it shoud not really have a side-effect... but you do some
`sc->add_control(cnode);` and `sc->push_string(arg);` etc.

I'm not familiar enough with the code yet, but those side-effects. Ah, the comment in the hpp seems to suggest you `add` it. So maybe:
`add_the_append_candidate_to_sc_if_<something>`
A bit long, but would be more helpful I think

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22537#discussion_r1907196154