[m-rev.] for post-commit review: fix a perf regression for --generate-dependencies

[Zoltan Somogyi]

On Thu, 14 Aug 2025 10:57:23 +1000, Julien Fischer <jfischer at opturion.com> wrote:
> On Thu, 14 Aug 2025 at 07:24, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:
> > --- a/library/digraph.m
> > +++ b/library/digraph.m
> > +    % reachable_vertices_from(G, FromVertex, ReachableVertices):
> > +    %
> > +    % Return the set of vertices that are reachable from FromVertex in G.
> 
> This comment should say whether the set returned includes FromVertex or not.

Done.

> > +    %
> > +:- pred reachable_vertices_from(digraph(T)::in, T::in, set(T)::out) is det.
> 
> Some alternative suggestions for names: reachable_vertex_set or
> reachable_vertices.

I think reachable_vertices_from is more descriptive than both of those.

This module has significant naming problems anyway. The main one is
the use of "vertex", when "node" is more commonly used, shorter,
and has a regular plural form :-( However, there are others as well, such as

- lookup_vertex and lookup_key each doing a job that could be better
  described by the *other* name,

- lookup_to and lookup_from being worse names than e.g. lookup_successors
  and lookup_predecessors would be,

- dfs, and therefore dfsrev, being horribly uninformative names, made worse by ...

- ... both being used *both* for a local operation starting at a given node, *and*
  for a global operation on the whole graph,

- and there are other predicate names that have two different-arity versions,
  such as "reduced".

For some of these, I could propose better names, but for the global versions
of dfs and dfsrev, I can't, because the documentation does not really tell me
what they are for :-(. I can  read the code, but that does not tell me *why*
it does what it does. Can anyone reading this email answer that question?

Zoltan.