[m-rev.] For Review: Bug fixes in constraints based mode analysis.

Richard Fothergill fothergill at gmail.com
Sun Mar 26 19:04:11 AEDT 2006


On 2/17/06, Julien Fischer <juliensf at cs.mu.oz.au> wrote:
>
> On Fri, 17 Feb 2006, Richard Fothergill wrote:
>
> > For review by anyone.
> >
> > Estimated hours taken: 35
> > Branch: main.
> >
> > Bugfixes for constraints based mode analysis (propagation solver).
> >

...
(All along I've been assuming you guys use "..." to indicate that some
of the quoted text has been cut, so that's what I'm using it for. Is
that right?)

>
> It would be good if you could document the current state of the mode
> constraint system somewhere, e.g. what's been done, what's working etc.
> (perhaps in the comment at the head of mode_constraints.m)
>

Ralph has made a similar request, but to be posted to the Mercury
Developers list. I'll do that, and then if you think it would be
appropriate for putting into a file just let me know and I'll do it.
It may be too time specific and therefore require a lot of updating as
things are changed.


> > Index: compiler/mode_constraints.m
> >

...

> > +        !.GoalExpr = switch(_SwitchVar, _CanFail, _Cases0),
> > +        unexpected(this_file, "switch")
> > +%         Goals0 = list.map(hlds_goal, Cases0),
> > +%         list.map_foldl2(ensure_unique_arguments_in_goal, Goals0, Goals,
> > +%             !SeenSoFar, !Varset, !Vartypes),
> > +%         Cases = list.map_corresponding('hlds_goal :=', Cases0, Goals),
> > +%         !:GoalExpr = switch(SwitchVar, CanFail, Cases)
>
> Why is this code commented out?  At the very least there should be an
> XXX comment explaining why.

It was there originally because it was suggested to me that someone
might want to run mode analysis after switch detection. I commented it
out because a lot of my other code did not work on switches, so it was
not doing anything. I've now deleted it, because it would not save
much work for someone expanding constriants based mode analysis to
switch detection, and is therefore just clutter.



>
> ...
>
> > Index: tests/valid/mc_extra_nonlocals.m
> > ===================================================================
> > RCS file: tests/valid/mc_extra_nonlocals.m
> > diff -N tests/valid/mc_extra_nonlocals.m
> > --- /dev/null 1 Jan 1970 00:00:00 -0000
> > +++ tests/valid/mc_extra_nonlocals.m  17 Feb 2006 06:22:22 -0000
> > @@ -0,0 +1,19 @@
> > +
> > +% This is a regression test. Some nonlocals sets in the compare predicate
> > +% generated for the polymorphic type below contained variables that
> > +% didn't appear in the goal. Constraints based mode analysis using the
> > +% propagation solve was incorrectly assuming that the goal consumed the
>
> s/solve/solver/
>

Done.

> > +% variable, and analysis was failing because of this.
> > +%
> > +% Constraints based mode analysis has been changed to ignore nonlocals
> > +% that don't appear in the goal, however the compare predicate for
> > +% this type may still have inaccurate nonlocals sets at the time of mode
> > +% analysis.
> > +
> > +:- module mc_extra_nonlocals.
> > +:- interface.
> > +
> > +:- type polymorphic(K)
> > +       --->    empty
> > +       ;       node(polymorphic(K)).
> > +
>
> Thanks for all your work on this over summer.
>

No problem. I've just had my first lazy Sunday afternoon this semester
and have spent it cleaning up all these loose ends (not to mention a
few merge conflicts that have arisen because I've left it too long) -
which should be evidence enough of how much I enjoy working on
Mercury.

-Richard

--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list