-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
lib: Add encapsulate, attrsets that have overlay-based private attrs in their closure #158781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -110,4 +110,94 @@ rec { | |||||
| fix' rattrs // { | ||||||
| ${extenderName} = f: makeExtensibleWithCustomName extenderName (extends f rattrs); | ||||||
| }; | ||||||
|
|
||||||
| /* | ||||||
| Creates an overridable attrset with encapsulation. | ||||||
|
|
||||||
| This is like `makeExtensible`, but only the `public` attribute of the fixed | ||||||
| point is returned. | ||||||
|
|
||||||
| Synopsis: | ||||||
|
|
||||||
| r = encapsulate (final@{extend, ...}: { | ||||||
|
|
||||||
| # ... private attributes for `final` ... | ||||||
|
|
||||||
| public = { | ||||||
| # ... returned attributes for r, in terms of `final` ... | ||||||
| inherit extend; # optional, don't invoke too often; see below | ||||||
| }; | ||||||
| }) | ||||||
|
|
||||||
| s = r.extend (final: previous: { | ||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I like this as a convention because it has a mnemonic: the reverse, "old this" can only be a description of one of the parameters and is therefore the wrong order. We should probably not reuse names of overlays, so that overlays and
|
||||||
|
|
||||||
| # ... updates to private attributes ... | ||||||
|
|
||||||
| # optionally | ||||||
| public = previous.public // { | ||||||
| # ... updates to public attributes ... | ||||||
| }; | ||||||
| }) | ||||||
|
|
||||||
| = Performance | ||||||
|
|
||||||
| The `extend` function evaluates the whole fixed point all over, reusing | ||||||
| no "intermediate results" from the existing object. | ||||||
| This is necessary, because `final` has changed. | ||||||
| So the cost is quadratic; O(n^2) where n = number of chained invocations. | ||||||
| This has consequences for interface design. | ||||||
| Although enticing, `extend` is not suitable for directly implementing "fluent interfaces", where the caller makes many calls to `extend` via domain-specific "setters" or `with*` functions. | ||||||
| Fluent interfaces can not be implemented efficiently in Nix and have very little to offer over attribute sets in terms of usability.* | ||||||
|
|
||||||
| Example: | ||||||
|
|
||||||
| # cd nixpkgs; nix repl lib | ||||||
|
|
||||||
| nix-repl> multiplier = encapsulate (self: { | ||||||
| a = 1; | ||||||
| b = 1; | ||||||
| public = { | ||||||
| r = self.a * self.b; | ||||||
|
|
||||||
| # Publishing extend makes the attrset open for any kind of change. | ||||||
| inherit (self) extend; | ||||||
|
|
||||||
| # Instead, or additionally, you can add domain-specific functions. | ||||||
| # Offer a single method with multiple arguments, and not a | ||||||
| # "fluent interface" of a method per argument, because all extension | ||||||
| # functions are called for every `extend`. See the Performance section. | ||||||
| withParams = args@{ a ? null, b ? null }: # NB: defaults are not used | ||||||
| self.extend (self: super: args); | ||||||
|
|
||||||
| }; | ||||||
| }) | ||||||
|
|
||||||
| nix-repl> multiplier | ||||||
| { extend = «lambda»; r = 1; withParams =«lambda»; } | ||||||
|
|
||||||
| nix-repl> multiplier.withParams { a = 42; b = 10; } | ||||||
| { extend = «lambda»; r = 420; withParams =«lambda»; } | ||||||
|
|
||||||
| nix-repl> multiplier3 = multiplier.extend (self: super: { | ||||||
| c = 1; | ||||||
| public = super.public // { | ||||||
| r = super.public.r * self.c; | ||||||
| }; | ||||||
| }) | ||||||
|
|
||||||
| nix-repl> multiplier3.extend (self: super: { a = 2; b = 3; c = 10; }) | ||||||
| { extend = «lambda»; r = 60; withParams =«lambda»; } | ||||||
|
|
||||||
| (*) Final note on Fluent APIs: While the asymptotic complexity can be fixed | ||||||
| by avoiding overlay extension or perhaps using it only at the end of the | ||||||
| chain only, one problem remains. Every method invocation has to produce | ||||||
| a new, immutable state value, which means copying the whole state up to | ||||||
| that point. | ||||||
|
|
||||||
| */ | ||||||
| encapsulate = layerZero: | ||||||
| let | ||||||
| fixed = layerZero ({ extend = f: encapsulate (extends f layerZero); } // fixed); | ||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Idea:
Suggested change
This allows the base layer (or subsequent layers) to pick a "merge function" that implements extension differently - ie not overlay-style extension, but something more suitable for the use case. |
||||||
| in fixed.public; | ||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Instead of Personally I actually prefer Similarly, imperative languages that implement functional features don't reinvent all names. For instance, when a language implements a functional list processing API, they use the simple function names that were supposed to be used with pure functions, even though that can't be enforced. If we do insist on something like |
||||||
|
|
||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting
extendinfinalis conceptually not valid. It should be a separate parameter.(It's the kind of code you end up with when trying to stick too close to past patterns, as in #119942)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this comes from the discussion in #157056 (comment)? If so, I think the concerns there have been cleared
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed your comment seems to be about the same kind of problem.
I think I was setting the bar too high when I started this thread. What's the alternative? Add an extra parameter?
Reserving a name in the attrset of protected attributes is not pretty, but gets the job done quite well, without burdening the user with extra syntax.
The extra parameter will cause it to look like this:
Doesn't seem like a usability improvement, especially if you're not actually going to use
extend.this.extendseems better now, even if it means that users can't use that name.That's probably for the best though, because we should having too many overriding mechanisms. If they do need to use that name, the problem is that they have one too many overriding mechanisms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose a more realistic alternative is
We don't have renaming for such parameters though, which can make nested use awkward, as seen with not-just-data submodules, which have the same problem.
rant and analogy with module system
Also before you know it, someone will want to extend that attrset, and we'll have
encapsulateWith { specialArgs = ...; }, even thoughsupercan fill the role ofspecialArgs. They're actually oddly similar.Overlays have
//for "lateral" composition (idk, is that term taken?), whereas the module system has option merging for that.extendModulesthen lines up withextends.extendModulesrelies on the lexical scope (ieconfig) to provide the "super" context, whereas in overlays it's an explicit parameter.specialArgsis effectively an extension of all modules' lexical scope, so it's a close analog ofsuper.This seems like an overreaction though.
The lack of nested "formals" (
{ this@{ foo, ... }, ... }: <body>) could be a feature though, because then it's not possible to do the equivalent of what's currentlylib.encapsulate ({ foo, ... }: <body>), which doesn't work, because such lambdas are strict (a phenomenon very relevant tolazyFunction, for some context).