[darcs-users] darcs patch: make an assumption in the pull code explicit

Jason Dagit dagit at codersbase.com
Wed Sep 9 19:23:51 UTC 2009


On Wed, Sep 9, 2009 at 7:34 AM, Eric Kow <kowey at darcs.net> wrote:

> On Tue, Sep 08, 2009 at 22:50:22 +0100, Ganesh Sittampalam wrote:
> > This patch is a step on the road towards adding type witnesses to
> Pull.lhs.
>
> Applied, thanks!
>
> > -      Sealed pw <- tentativelyMergePatches repository "pull" merge_opts
> > -                   (reverseRL $ head $ unsafeUnRL us') to_be_pulled
> > +      Sealed pw <- case us' of
> > +                     h_us :<: NilRL -> tentativelyMergePatches
> repository "pull" merge_opts
> > +                                        (reverseRL h_us) to_be_pulled
> > +                     _ -> impossible -- we believe that
> get_common_and_uncommon should guarantee this,
> > +                                     -- at least in this case. Error out
> if we're wrong, so that
> > +                                     -- we find out. An alternative
> would be to do a concatRL of the whole
> > +                                     -- us' list, but the code
> originally just took the head, and so we
> > +                                     -- might instead introduce some
> subtle bug by doing a concat.
>
> Blowing up explicitly and with an explanation why we're surprised sounds
> like the kind of thing we should be doing systematically.  Hlint?
>

I discussed this with Ganesh over IRC as well.  I had also discussed this
case with David in the past.  Additionally, if you inspect the definition of
get_common_and_uncommon it appears to always add (:<:NilRL) to what it
returns.  The main problem is just that the type of get_common_and_uncommon
doesn't express this (really it can't), so ultimately we need to update
get_commonn_and_uncommon to have the correct type and stop adding (:<:NilRL)
to the end of its output.

Anyway, I'm almost 100% certain we can't hit the impossible case.  I seem to
recall that David had asked me to hold off and refactoring it simply because
my focus was elsewhere previously (one simple change at a time).  Once the
type-witnesses are 100% I think we should revisit get_common_and_uncommon.
Actually, I feel like a lot of the Depends module needs modernization and
cleanup.  We still see bugs there way more regularly than we should and the
way the algorithms are expressed there are too many unsafeCoercePs in that
module.

I think I'll go make a bug ticket for this future refactor.

Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20090909/d6b65db1/attachment.htm>


More information about the darcs-users mailing list