[PATCH] sortdict: fix .pop() to return a value

[Yuya Nishihara]

On Sun, 9 Apr 2017 19:50:26 -0700, David Soria Parra wrote:
> On Sun, Apr 09, 2017 at 10:10:01PM +0900, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya at tcha.org>
> > # Date 1491706629 -32400
> > #      Sun Apr 09 11:57:09 2017 +0900
> > # Node ID 48a7a1f77a9489e3c4b5f862243782ceae80eaf9
> > # Parent  9259cf823690e4fcd34a4d2ecd57ced2060d2b3d
> > sortdict: fix .pop() to return a value
> > 
> > My future patch will need it.
> 
> Generally this looks fine to me.
> 
> > diff --git a/mercurial/util.py b/mercurial/util.py
> > --- a/mercurial/util.py
> > +++ b/mercurial/util.py
> > @@ -555,11 +555,11 @@ class sortdict(dict):
> >          dict.__delitem__(self, key)
> >          self._list.remove(key)
> >      def pop(self, key, *args, **kwargs):
> > -        dict.pop(self, key, *args, **kwargs)
> >          try:
> >              self._list.remove(key)
> >          except ValueError:
> >              pass
> > +        return dict.pop(self, key, *args, **kwargs)
> 
> Note that this breaks the insert/delete asymmetry. We usually insert into the
> _list first and then add it to the dictionary and the current version of pop
> deletes from the dictionary first and then from the list ensuring an
> 
> add to list
> add to dict
> del from dict
> del from list
> 
> order. Your patch breaks this.

Yes, but anyway sortdict doesn't guarantee strong exception safety nor thread
safety, so the order shouldn't matter. The only practical reason why
__delitem__() calls dict first is to raise KeyError.