<br><br><div class="gmail_quote">On Wed, Sep 9, 2009 at 7:34 AM, Eric Kow <span dir="ltr">&lt;<a href="mailto:kowey@darcs.net">kowey@darcs.net</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">On Tue, Sep 08, 2009 at 22:50:22 +0100, Ganesh Sittampalam wrote:<br>
&gt; This patch is a step on the road towards adding type witnesses to Pull.lhs.<br>
<br>
</div>Applied, thanks!<br>
<br>
&gt; -      Sealed pw &lt;- tentativelyMergePatches repository &quot;pull&quot; merge_opts<br>
&gt; -                   (reverseRL $ head $ unsafeUnRL us&#39;) to_be_pulled<br>
&gt; +      Sealed pw &lt;- case us&#39; of<br>
&gt; +                     h_us :&lt;: NilRL -&gt; tentativelyMergePatches repository &quot;pull&quot; merge_opts<br>
&gt; +                                        (reverseRL h_us) to_be_pulled<br>
&gt; +                     _ -&gt; impossible -- we believe that get_common_and_uncommon should guarantee this,<br>
&gt; +                                     -- at least in this case. Error out if we&#39;re wrong, so that<br>
&gt; +                                     -- we find out. An alternative would be to do a concatRL of the whole<br>
&gt; +                                     -- us&#39; list, but the code originally just took the head, and so we<br>
&gt; +                                     -- might instead introduce some subtle bug by doing a concat.<br>
<br>
Blowing up explicitly and with an explanation why we&#39;re surprised sounds<br>
like the kind of thing we should be doing systematically.  Hlint?<br></blockquote><div><br>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 (:&lt;:NilRL) to what it returns.  The main problem is just that the type of get_common_and_uncommon doesn&#39;t express this (really it can&#39;t), so ultimately we need to update get_commonn_and_uncommon to have the correct type and stop adding (:&lt;:NilRL) to the end of its output.<br>
<br>Anyway, I&#39;m almost 100% certain we can&#39;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.<br>
<br>I think I&#39;ll go make a bug ticket for this future refactor.<br><br>Jason<br></div></div>