[darcs-devel] witnesses changes

[Ganesh Sittampalam]

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>> 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/>).
> 
>     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