On Tue, 30 Sep 2025 08:51:25 GMT, Marc Chevalier <[email protected]> wrote:
>> src/hotspot/share/opto/cfgnode.cpp line 1482:
>>
>>> 1480: }
>>> 1481: {
>>> 1482: Node* uin = unique_constant_input_recursive(phase);
>>
>> I think the original format was fine but I have no strong opinion.
>
> I wouldn't fight too hard for it, but when trying things, it was necessary
> (since I wanted different types for `uin`), but I also think it helps to see
> that it is not useful too long, and I can mess with it without having to look
> at what comes after. In this case, indeed, it feels a bit coming out of
> nowhere at the end...
>
> In C++17, I'd do:
> ```c++
> if (Node* uin = unique_constant_input_recursive(phase); uin != nullptr) {
> return uin;
> }
>
> one could also do
> ```c++
> if (Node* uin = unique_constant_input_recursive(phase)) {
> return uin;
> }
>
> if we are not afraid of implicit conditions (but we are, and I'm fine with
> it!).
Right but now that both `uin` have the same type, the scoping seems unnecessary.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1640#discussion_r2393507487
