[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