[m-rev.] for post-commit review: begin addressing issues in the ranges module

Zoltan Somogyi zoltan.somogyi at runbox.com
Wed Jan 8 17:35:36 AEDT 2025



On Wed, 8 Jan 2025 17:26:39 +1100, Julien Fischer <jfischer at opturion.com> wrote:
> On Mon, 6 Jan 2025 at 16:43, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:
> >
> >
> > On Mon, 6 Jan 2025 15:22:26 +1100, Julien Fischer <jfischer at opturion.com> wrote:
> > > > Of course, list.member does it, but it came from Prolog.
> > > > I would prefer it if the new predicates, nondet_member and
> > > > nondet_range_member, had the ranges arg before the int arg or args.
> > >
> > > nondet_member is *not* a new predicate. Altering its argument ordering will be
> > > a breaking change.
> >
> > I missed that.
> >
> > > I'm not opposed to doing so, although I will need to take a
> > > look at how extensively it is currently being used beforehand.
> >
> > In that case, what we should probably do is introduce a new
> > predicate with the in, out order, and mark the old predicate obsolete
> > in its favor. The new one should have a more descriptive name than
> > "nondet_member". Would "return_members" or "return_members_one_by_one"
> > do?
> 
> I have a preference for nondet predicates like these having "nondet" somewhere
> in their names.  Obviously, it makes no difference to the compiler but
> it does help
> to make it obvious to the programmer where calls that introduce choice
> points are.

I agree with the need for that warning. I just don't think the presence of "nondet"
in the name makes the *rest* of the name automatically meaningful :-(

In this case, the "member" part is fine, and if have as consistent a
convention for "nondet_" prefixes as we have for "det_" prefixes (on exception
throwing versions of semidet operations), then nondet_member works.
I just don't think that name works all that well in the absence of that convention.

> I've had a look through G12 (and some other Opturion projects) and
> there are very
> few calls to either of these predicates.  I can't speak for other
> users, but I think we
> can probably get away with making a breaking change here.

I am not objecting to a breaking change either, but then again,
I have been replacing needlessly-nondet code in the compiler
first det/semidet code for ages.

> > > The usual convention in this module is that predicates with "range" in their
> > > name operate on ranges (e.g. range_foldl) and those without it operate on
> > > individual values in the set.
> >
> > Is that documented anywhere? If not, it should be.
> 
> I will add some documentation  for it.

Thanks.

Zoltan.





More information about the reviews mailing list