RFR: 8350864: C2: verify structural invariants of the Ideal graph [v3]

[Marc Chevalier]

On Mon, 25 Aug 2025 14:03:58 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Benoît's comments
>
> src/hotspot/share/opto/graphInvariants.cpp line 207:
> 
>> 205:   }
>> 206:   bool (Node::*_type_check)() const;
>> 207: };
> 
> You could probably generalize this with a callback approach. And then one concrete implentation is the one that does the type check. Just an idea.

Seems overengineered to me. The callback version would be similarly long as this. The user that must provide the callback will also be similarly long. It makes the logic unnecessarily complicated to me. Of course, everything boils down to a function that takes a node and perform a specific check, but then, this generalized version does nothing significant but calling the callback. The concrete implementation will just have all the same logic, but in a callback passed to another method instead of having it as a first class method...

If I don't have an adapter class that would only check type but I leave that at instanciation time, the code would look like

NodeCallback([](const Node* n) { return n->is_Region(); })

instead of

NodeClass(&Node::is_Region)

which is unreadable. That's the point of patterns: it makes easy to understand the shape, otherwise, one can just write normal, manual traversal, which is all powerful.

It was also discussed above that something like the `NodeCallback` could exist for when we need something that can't be expressed simply, but:
- will it ever happen?
- NodeCallback doesn't even provide a useful error messages, we would also need a callback to craft it (or make the one callback more complicated, that would be pretty much the content of `NodeClass::check`)
- I'm not willing to make the common kind of patterns ugly for a rare usecase.

And as for implementing `NodeClass` from a hypothetical `NodeCallback`, what would be the concrete benefits? (kinda the first paragraph again: all the logic in the callback, and NodeCallback doing nothing).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26362#discussion_r2321388003