> > +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