[m-rev.] for review: Implement modechecking of coerce more efficiently.
Peter Wang
novalazy at gmail.com
Wed May 5 17:29:38 AEST 2021
On Wed, 05 May 2021 17:03:05 +1000 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
>
> > > > +:- pred modecheck_coerce_from_ground_make_inst(module_info::in, tvarset::in,
> > > > + is_live::in, uniqueness::in, mer_type::in, mer_type::in,
> > > > + maybe(mer_inst)::out) is det.
> > > > +
> > > > +modecheck_coerce_from_ground_make_inst(ModuleInfo, TVarSet, LiveX, UniqX,
> > > > + TypeX, TypeY, MaybeInstY) :-
> > > > + ( if TypeX = TypeY then
> > > > + % This should be a common case.
> > > > + UniqY = make_result_uniq(LiveX, UniqX),
> > > > + InstY = ground(UniqY, none_or_default_func),
> > > > + MaybeInstY = yes(InstY)
> > > > + else if check_is_subtype(ModuleInfo, TVarSet, TypeX, TypeY) then
> > > > + set.init(SeenTypes0),
> > > > + modecheck_coerce_from_ground_make_inst_2(ModuleInfo, TVarSet,
> > > > + LiveX, UniqX, SeenTypes0, TypeX, TypeY, InstY),
> > > > + MaybeInstY = yes(InstY)
> > > > + else
> > > > + MaybeInstY = no
> > > > + ).
> > >
> > > If I am not mistaken, this is called from exactly one place. I would inline it there,
> > > and remove the _2 from the name of the next predicate.
> > >
> >
> > It's called recursively as well.
>
> There is no direct recursion in the code above. When I do a search for
> modecheck_coerce_from_ground_make_inst in your message, I get the
> declaration and the definition above, the call from modecheck_coerce_make_inst,
> and all the other matches have a _2 at the end.
Sorry, somehow I got it in my mind you were suggesting to inline the _2
predicate. Yes, modecheck_coerce_make_inst is only called from one
place, and it's the _2 predicate that is recursive.
I prefer how it's organised now. I think the _2 predicate just needs a
better name.
Peter
More information about the reviews
mailing list