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

[Martin von Zweigbergk]

On Jun 5, 2017 6:22 AM, "Yuya Nishihara" <yuya at tcha.org> wrote:

On Sun, 4 Jun 2017 17:36:04 -0700, Jun Wu wrote:
> Excerpts from Yuya Nishihara's message of 2017-06-04 11:41:26 +0900:
> > On Sat, 3 Jun 2017 20:02:28 -0400, Augie Fackler wrote:
> > > 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.
>
> I think Python is fundamentally unfriendly for marking data as immutable:
>
>   - everything is mutable by default (unlike Rust)
>   - set -> frozenset, list -> tuple are at least O(N)
>
> It feels like if "const" in C++ is not just a compiler hint but has
runtime
> penalty, would you still use "const"?
>
> I guess my answer is no. Even if we fix baseset, it's likely that we still
> have many places need to be fixed, like obsstore._all.
>
> Therefore I prefer the documentation way to "fix" this issue.

Okay. Then, my two cents is naming the function with underscore-prefix
(e.g. _asset()) to trigger extra attention.


I agree with Yuya. However, given our lack of underscores, "asset" is not
the best name.

And please add at least one call site if you resend.


_______________________________________________
Mercurial-devel mailing list
Mercurial-devel at mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170605/64daf894/attachment.html>