[PATCH V2] smartset: add a "toset" method
Yuya Nishihara
yuya at tcha.org
Sun Jun 4 02:41:26 UTC 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