OK, LGTM

On Fri, Jun 6, 2014 at 10:27 AM, Klaus Aehlig <[email protected]> wrote:

> > > +recollectLocksData (allLocks, pending) _ _  =
> > > +  let getPending lock = pending >>= \(_, cid, req) ->
> > > +        let req' = filter ((==) lock . lockName . lockAffected) req
> > > +        in case () of
> > > +          _ | any ((==) (Just OwnExclusive) . lockRequestType) req'
> > > +              -> [(cid, OwnExclusive)]
> > > +          _ | any ((==) (Just OwnShared) . lockRequestType) req'
> > > +              -> [(cid, OwnShared)]
> > > +          _ -> []
> > >
> >
> > IIUC this piece of code computes the maximum ownership level. Since we
> > already have the proper ordering on Maybe OwnerState, we could add
> > something like this (perhaps to Allocation.hs)
> >
> >   maxOwnerState :: [Maybe OwnerState] -> Maybe OwnerState
> >   maxOwnerState = maximum . (Nothing :) -- ensure it's safe on empty
> lists
> >
> > and then just call it on req' combined with maybeToList.
>
> Well, yes, technically it is a maximum---even though I was thinking of it
> as a very defensive way of doing a case distinction on a
> zero-or-one--element
> list.
>
> The reason why I'm hesitant to code it using maximum is that the order on
> OwnerState was introduced for technical reasons (so that you can put it
> into
> a set) and so far we never assumed any particular properties of this order
> besides being a linear order.
>
> > In any case LGTM, no need to resend.
>
> So I would prefer to leave it as is.
>
> Thanks,
> Klaus
>
> --
> Klaus Aehlig
> Google Germany GmbH, Dienerstr. 12, 80331 Muenchen
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores
>

Reply via email to