Skip to content

Add functionInfo builtin#8961

Open
vkryachko wants to merge 1 commit into
NixOS:masterfrom
vkryachko:functionInfo
Open

Add functionInfo builtin#8961
vkryachko wants to merge 1 commit into
NixOS:masterfrom
vkryachko:functionInfo

Conversation

@vkryachko

Copy link
Copy Markdown
Contributor

This function returns full information about the function, including its formal arguments, whether formals have an ellipse, and optionally the name of the @-pattern argument.

Motivation

It's sometimes useful to know if the function uses @-pattern to know if it's safe to call it with callPackage-style, i.e. only passing its "formals".

Context

Useful in #254212.
A similar change has previously bee proposed as well #7317
It's also a less hacky solution to #194992

Priorities

Add 👍 to pull requests you find important.

This function returns full information about the function, including its
formal arguments, whether formals have an ellipse, and optionally the
name of the @-pattern argument.

Motivation: it's sometimes useful to know if the function uses @-pattern
to know if it's safe to call it with callPackage-style, i.e. only
passing its "formals" to it. i.e. useful in
[254212](NixOS/nixpkgs#254212).
A similar change has previously bee proposed as well NixOS#7317
@vkryachko vkryachko requested a review from edolstra as a code owner September 10, 2023 09:13
@github-actions github-actions Bot added the with-tests Issues related to testing. PRs with tests have some priority label Sep 10, 2023
@roberth

roberth commented Sep 11, 2023

Copy link
Copy Markdown
Member

#8908 also implements most of the reflection logic proposed here, as separate primops.

@roberth roberth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A mostly equivalent change is proposed as part of

Differences to consider

  • return an attrset or have multiple functions?
    • 8908 is more consistent with functionArgs and avoids committing the language to what I consider to be a bad name, formals
    • this could be considered to promote consistency by having a single format for the combined info
  • use semantically meaningful names or syntactic names? The latter make interoperability with a possibly different syntax ugly. (think cue or python, or functions defined through the bindings)
  • expose the difference between a@{...}: and a:? the prior of which is strict and may perhaps warrant a custom error or warning in some cases

Comment thread src/libexpr/primops.cc
// !!! should optimise booleans (allocate only once)
formals.alloc(i.name, i.pos).mkBool(i.def);
auto attrs = state.buildBindings(3);
if(args[0]->lambda.fun->arg) attrs.alloc("arg").mkString(state.symbols[args[0]->lambda.fun->arg]);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should not expose this information. It is not needed, and it will introduce unnecessary coupling that users will have to be aware of when renaming what is otherwise a completely internal name.

Comment thread src/libexpr/primops.cc
Comment on lines +2812 to +2814
for (auto & i : args[0]->lambda.fun->formals->formals)
// !!! should optimise booleans (allocate only once)
formals.alloc(i.name, i.pos).mkBool(i.def);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be common with the functionArgs primop and should be factored out.

Comment thread src/libexpr/primops.cc
auto attrs = state.buildBindings(3);
if(args[0]->lambda.fun->arg) attrs.alloc("arg").mkString(state.symbols[args[0]->lambda.fun->arg]);
attrs.alloc("ellipsis").mkBool(args[0]->lambda.fun->formals->ellipsis);
attrs.alloc("formals").mkAttrs(formals);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe formals is an unfortunate name, and it has so far only been part of documentation, and not the language itself. This would be the last chance to fix that mistake.
args would match functionArgs. Will come back to this.

@roberth

roberth commented Mar 28, 2024

Copy link
Copy Markdown
Member

I've written #10350, to be triaged by the Nix team.

@vkryachko

Copy link
Copy Markdown
Contributor Author

I've written #10350, to be triaged by the Nix team.

Cool, thanks @roberth

@edolstra edolstra changed the title Add functionInfo builtin. Add functionInfo builtin Nov 27, 2024
@nixos-discourse

Copy link
Copy Markdown

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-11-27-nix-team-meeting-minutes-198/56691/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants