Skip to content

Add builtins.unsafeGetLambdaPos #3912

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

Closed
wants to merge 2 commits into from
Closed

Add builtins.unsafeGetLambdaPos #3912

wants to merge 2 commits into from

Conversation

lf-
Copy link
Member

@lf- lf- commented Aug 8, 2020

This is useful for a potential pure-Nix implementation of #3904 as unsafeGetAttrPos does not appear to be usable for finding lambdas that are defined by import, for example.

One thing I noticed while implementing this is that the way that Nix exposes Pos for things in the REPL as a set is surprising:

nix-repl> s = {a = 2;}                             

nix-repl> builtins.unsafeGetAttrPos "a" s          
{ column = 3; file = " {a = 2;}\n"; line = 1; }

nix-repl> f = a: "2"

nix-repl> builtins.unsafeGetLambdaPos f             
{ column = 2; file = " a: \"2\"\n"; line = 1; }

in that it doesn't report whether the file attribute is actually a file or arbitrary Nix source. It is unclear whether it's possible to rely on there being a leading space to determine whether a given file is actually Nix code.

@Ericson2314
Copy link
Member

I would rather we had :doc without this. I'm not eager to have more builtin.unsafe*, though pure eval mode does help mitigate.

@matthewbauer
Copy link
Member

I would rather we had :doc without this. I'm not eager to have more builtin.unsafe*, though pure eval mode does help mitigate.

I don't think pure eval mode disallows any of the unsafe builtins.


Do we need to restrict this to lambdas? We should have position information for most Nix expressions.

@lf-
Copy link
Member Author

lf- commented Aug 8, 2020

I believe that the reason it's called unsafe is because it basically guarantees that your function is impure as it then starts depending on the exact source making the derivation. Perhaps it should be named differently, but then it's inconsistent. I want this functionality for debugging mostly, anyway, and I can see it being potentially useful outside of the REPL in tracing or other places.

I do not believe it is a risk for restricted evaluation mode because you'd have to first evaluate the file that's not in the Nix store before you get any information on it.

As for restricting it to lambdas, that's a fantastic question I do not know the answer to and I was mostly staring at lambdas too hard to notice.

@lf-
Copy link
Member Author

lf- commented Aug 15, 2020

@matthewbauer I looked into it. If we just have the value, it is not possible to get the position of things other than lambdas. See src/libexpr/value.hh in struct Value: the only value types that appear to have any metadata to speak of in there are lambdas.

It might be possible to take advantage of lazy evaluation in some way to achieve this, since perhaps we might have the appropriate context still existing prior to the forceValue call, but I don't know enough about Nix internals to implement this idea.

I have also made it so :doc can be implemented directly as a Nix plugin with #3934.

If there isn't any other proposed use of unsafeGetLambdaPos besides hypothetically building an inferior version of the now-existing nix-doc nix plugin that just got into nixpkgs (that uses essentially this code but natively from C++ and does the docs stuff in-process), I think we should abandon this PR.

@lf- lf- closed this Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants