[PATCH V2] smartset: add a "toset" method

Yuya Nishihara yuya at tcha.org
Sat Jun 3 22:41:26 EDT 2017


On Sat, 3 Jun 2017 20:02:28 -0400, Augie Fackler wrote:
> 
> > On Jun 3, 2017, at 7:29 PM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
> > 
> >>> +    def toset(self):
> >>> +        return self._set
> >> 
> >> It freaks me out just a little (maybe too much Rust today?) to leak self._set mutably like this. What do you think of making a copy? Should we just strongly admonish in the “convert to a set” docstring that callers of toset() _must not_ mutate the returned set?
> > 
> > Note: For the target code, copying sets is can be multiple percent of the total run time.
> 
> Okay, it sounds like we should document that the API contract is that clients must not mutate the returned set. Does that sound workable to everyone?

Can't we change self._set to frozenset? smartset internals are cache heavy,
mutating it would lead to subtle bugs.


More information about the Mercurial-devel mailing list