[darcs-devel] witnesses changes

Aditya bsrkaditya at gmail.com
Wed Jun 27 07:40:37 UTC 2012


Thanks Ganesh, that worked!

On Wed, Jun 27, 2012 at 12:20 PM, Ganesh Sittampalam <ganesh at earth.li>wrote:

> Hi,
>
> OK, so the problem is that there's a mismatch between the expected end
> contexts on the two sides of the if statement - in the patch index case,
> the end context is the recorded state of the repository wR - as seen in
> the signature of annotate itself - and in the other it's some unknown
> context wX coming from the unwrapping of 'Sealed patches' earlier on.
>
> Since the old code was also using wX, and patches isn't used inbetween
> the 'Sealed patches' match and this point in the code, I would try just
> moving the unwrapping further down - i.e. keep patches as Sealed, and
> unwrap the Sealed when binding ans_patches from this if statement. That
> would require adding a seal to the patch index case somewhere.
>
> It also seems a bit fishy that in the patch index case, patches is being
> calculated then ignored, but perhaps the magic of lazy evaluation makes
> that ok.
>
> Cheers,
>
> Ganesh
>
> On 27/06/2012 07:18, Aditya wrote:
> > Here is the error if I remove unsafeCoerce on repository for the call to
> > getPatches:
> >
> >         src/Darcs/UI/Commands/Annotate.hs:177:63:
> >
> >             Could not deduce (wR ~ wX)
> >
> >             from the context (RepoPatch p, ApplyState p ~ Tree)
> >
> >               bound by the type signature for
> >
> >                          annotate' :: (RepoPatch p, ApplyState p ~ Tree)
> =>
> >
> >                                       [DarcsFlag] -> [String] ->
> >         Repository p wR wU wR -> IO ()
> >
> >               at src/Darcs/UI/Commands/Annotate.hs:(116,1)-(182,62)
> >
> >               `wR' is a rigid type variable bound by
> >
> >                    the type signature for
> >
> >                      annotate' :: (RepoPatch p, ApplyState p ~ Tree) =>
> >
> >                                   [DarcsFlag] -> [String] -> Repository
> >         p wR wU wR -> IO ()
> >
> >                    at src/Darcs/UI/Commands/Annotate.hs:116:1
> >
> >               `wX' is a rigid type variable bound by
> >
> >                    a pattern with constructor
> >
> >                      Sealed :: forall (a :: * -> *) wX. a wX -> Sealed a,
> >
> >                    in a pattern binding in
> >
> >                         'do' block
> >
> >                    at src/Darcs/UI/Commands/Annotate.hs:146:4
> >
> >             Expected type: Repository p wX wU wX
> >
> >               Actual type: Repository p wR wU wR
> >
> >             In the first argument of `getPatches', namely `repository'
> >
> >             In the second argument of `fmap', namely
> >
> >               `getPatches repository path patch_id'
> >
> >
> >
> > On Wed, Jun 27, 2012 at 11:36 AM, Ganesh Sittampalam <ganesh at earth.li
> > <mailto:ganesh at earth.li>> wrote:
> >
> >     What error do you get without the unsafeCoerce on repository?
> >
> >     On 27/06/2012 06:58, Aditya wrote:
> >     > Hi Heffalump,
> >     >  Thanks for the review! I will encapsulate the functions that use
> >     > unsafeCoerce.
> >     >  I need you help with unsafeCoerce in a different place, however.
> >     > Please look at
> >     > Darcs.UI.Commands.Annotate.
> >
> http://darcsden.com/bsrkaditya/darcs-patch-index-rebase-2012-06-17/browse/src/Darcs/UI/Commands/Annotate.hs
> >     > I have ported patch-index annotate into the module.
> >     > (patch:
> >
> http://darcsden.com/bsrkaditya/darcs-patch-index-rebase-2012-06-17/patch/20120627050014-ae621
> )
> >     > I am using unsafeCoerce on repository, and I want to avoid using
> it.
> >     > (This was not necessary before combining the two annotates)
> >     >
> >     > Thanks,
> >     > Aditya.
> >     >
> >     > On Tue, Jun 26, 2012 at 11:47 PM, Ganesh Sittampalam
> >     <ganesh at earth.li <mailto:ganesh at earth.li>
> >     > <mailto:ganesh at earth.li <mailto:ganesh at earth.li>>> wrote:
> >     >
> >     >     Hi Aditya,
> >     >
> >     >     I had a look at the witnesses changes you mentioned on IRC
> >     after our
> >     >     meeting (in your branch at
> >     >     darcsden.com/bsrkaditya/darcs-patch-index-rebase-2012-06-17/
> >     <http://darcsden.com/bsrkaditya/darcs-patch-index-rebase-2012-06-17/
> >
> >     >
> >     <http://darcsden.com/bsrkaditya/darcs-patch-index-rebase-2012-06-17/
> >).
> >     >
> >     >     It's good to see at least some of the Sealed2 stuff gone - I
> >     lost track
> >     >     of whether it's all gone yet, but the end goal should
> >     definitely be to
> >     >     avoid any new uses of [Sealed2 ...], although I think that the
> >     changes
> >     >     code might already have a bit of that before the patch index
> >     was added
> >     >     so obviously it's fine for that to remain for now.
> >     >
> >     >     I forgot to emphasise that use of unsafeCoerce on witnesses in
> >     darcs is
> >     >     supposed to be tightly constrained, to reduce the risk of
> >     using it in a
> >     >     way that's actually dangerous. Having getElems and dropFLFL
> use it
> >     >     directly is therefore not a good idea, because those functions
> >     are quite
> >     >     general could be used in a different context to patch index
> >     filtering.
> >     >
> >     >     In the meeting I suggested using filterFLFL to achieve this,
> >     with an
> >     >     unsafe function being passed in that works only on the basis
> >     of whether
> >     >     something is excluded from the patch index or not - but
> >     looking at the
> >     >     patch index code a bit more I see that may be difficult as it
> >     relies on
> >     >     a positional analysis which would be tricky to do as a filter.
> >     >
> >     >     Given that, it's probably best to encapsulate the unsafeCoerce
> >     within
> >     >     one or two functions that read the repository and the patch
> index
> >     >     together (two might be needed given what you said about
> >     needing an FL in
> >     >     one case and a PatchSet in another). Overall the goal should
> >     be that the
> >     >     unsafety can't be easily used by outside callers to do
> something
> >     >     completely different.
> >     >
> >     >     In the long run I would like to have as simple an abstraction
> as
> >     >     possible for doing this (hence my suggestion of viewing this
> as a
> >     >     filter), but overall safety is the primary goal and simplicity
> is
> >     >     secondary.
> >     >
> >     >     BTW it's better to use unsafeCoerceP than unsafeCoerce; it has
> >     a type
> >     >     that means it's less likely to be used by accident on the
> >     wrong kind of
> >     >     thing.
> >     >
> >     >     Feel free to ask more questions by email or arrange another
> >     morning IRC
> >     >     meeting.
> >     >
> >     >     Cheers,
> >     >
> >     >     Ganesh
> >     >
> >     >
> >     >
> >     >
> >     > --
> >     > BSRK Aditya
> >
> >
> >
> >
> >
> > --
> > BSRK Aditya
>
>
>


-- 
BSRK Aditya
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/darcs-devel/attachments/20120627/87f575e8/attachment.html>


More information about the darcs-devel mailing list