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

Yuya Nishihara yuya at tcha.org
Mon Jun 5 09:16:15 EDT 2017


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.


More information about the Mercurial-devel mailing list