On Wed, Apr 15, 2026 at 2:46 AM Andrew Pinski via Gcc <[email protected]> wrote: > > Hi all, > Right now make_forwarder_block takes two callbacks functions. The > second one (new_bb_cbk) looks unused; from what I can tell the last > usage was r0-78960-g89f8f30f356532 which allowed it to be NULL even > :). So removing that is not an issue. > > The other (redirect_edge_p), is what I am getting at here. Currently > the callback almost always (except in cleanup_tree_cfg_noloop), uses a > global variable to check against for the return value. > > E.g. > mfb_redirect_edges_in_set in cfgloop.cc has a `hash_set<edge> *` to > check against to return true. > mfb_keep_just in cfgloopmanip.cc has a edge to see if we should return false. > > Would it be ok to use `void*` here for the callback or should we do > something more type safe like a class with virtual function? > > I am adding another use of make_forwarder_block and I am not a fan of > the global which is why I am asking. > > The virtual function case would be something like: > struct redirect_edge_p > { > virtual bool callback (edge) = 0; > }; > > and then each place would do something like: > struct mfb_keep_just : redirect_edge_p > { > edge mfb_kj_edge; > virtual bool callback (edge e) overload { return e != mfb_kj_edge; } > }; > > What do others think? Note a lambda won't work here as that is a local > type only.
Not being much into C++ patterns, void * is what we use elsewhere, so it would be a no-brainer to approve it this way. Sth type-safe would be nice, but only if it does not make the whole thing a template. The class with virtual function idea might work, it's not much more typing, but I'd like to see C++ folks chime in for approval. Richard. > > Thanks, > Andrew
