[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