Skip to content
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

Explain why so many builtins are available in global scope #7290

Open
2 tasks done
fricklerhandwerk opened this issue Nov 11, 2022 · 6 comments
Open
2 tasks done

Explain why so many builtins are available in global scope #7290

fricklerhandwerk opened this issue Nov 11, 2022 · 6 comments
Labels
bug documentation language The Nix expression language; parser, interpreter, primops, evaluation, etc

Comments

@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Nov 11, 2022

Problem

As a documentation maintainer I want to guide users into understanding the Nix language.

What's confusing is that quite many builtins are also available in the global scope for no apparent reason, although the manual says that this global attribute set is there to avoid polluting the namespace:

  • abort
  • baseNameOf
  • builtins (yes, recursively)
  • derivation
  • derivationStrict
  • dirOf
  • fetchGit
  • fetchMercurial
  • fetchTarball
  • fromTOML
  • import
  • isNull
  • map
  • placeholder
  • removeAttrs
  • scopedImport
  • throw
  • toString

@sternenseemann: a list including __* builtins

It would not suffice saying that these are for backwards compatibility, as some of them were introduced after Nix 2.0.

Checklist

Proposal

Be deliberate about it. Only make the following builtins available globally and tell why:

For general convenience:

  • import
  • map
  • toString

For convenience when testing and debugging:

  • abort
  • throw
  • derivation

There may be another group for backwards compatibility, but that should, with appropriate grace periods:

  • throw a warning
  • throw an error
  • cease existing
@fricklerhandwerk fricklerhandwerk added bug documentation language The Nix expression language; parser, interpreter, primops, evaluation, etc labels Nov 11, 2022
@sternenseemann
Copy link
Member

Only make the following builtins available globally

Disagree. This is going to unnecessarily break downstream code outside of nixpkgs. Because these shortcuts are available, people are using them. I think it is also imperative that the Nix language stays backwards as much as possible and it is unnecessary to jeopardize this by removing builtins.

Your recommendations seem fine. We can probably go a similar way as with the URL syntax and add an experimental feature for warnings if you violate those recommendations.

Removing the builtin aliases that start with two underscores seems like a no brainer (if some research was done if there's real world usage), but is actually a bit tricky:

  • __nixPath and __findFile are currently required by the expression <…> syntax desugars to. Shadowing __findfile allows overloading the <…> syntax. I would argue that this is actually useful.
  • Shadowing __sub, __mul and __div allows overloading the -, * and / operators, respectively. Note that __add can't be used to overload +.
  • Shadowing __lessThan can be used to overload <, >, <= and >= which all desugar to sometimes negated calls to __lessThan.

@oxalica
Copy link
Contributor

oxalica commented May 14, 2023

Only make the following builtins available globally

Disagree. This is going to unnecessarily break downstream code outside of nixpkgs. Because these shortcuts are available, people are using them. I think it is also imperative that the Nix language stays backwards as much as possible and it is unnecessary to jeopardize this by removing builtins.

Your recommendations seem fine. We can probably go a similar way as with the URL syntax and add an experimental feature for warnings if you violate those recommendations.

Removing the builtin aliases that start with two underscores seems like a no brainer (if some research was done if there's real world usage), but is actually a bit tricky:

I'd think these as unreliable implementation details.

* `__nixPath` and `__findFile` are currently required by the expression `<…>` syntax desugars to. Shadowing `__findfile` allows overloading the `<…>` syntax. I would argue that this is actually useful.

* Shadowing  `__sub`, `__mul` and `__div` allows overloading the `-`, `*` and `/` operators, respectively. Note that `__add` can't be used to overload `+`.

* Shadowing `__lessThan` can be used to overload `<`, `>`, `<=` and `>=` which all desugar to sometimes negated calls to `__lessThan`.

IRCC, identifiers starting with __ are considered language reserved, no matter as reference or attribute names, similar to C++. Users are not supposed to use unless documented ones like __functor. It may cause unexpected and unrelieable behaviors, just like partial-working operator overloading you've mentioned, let alone new features gained in the future may break the code using them.

It's good to discourage undocumented identifiers starting with __ in any place, hide them in completions, and state that there are no future guarantees for their usages in documentation.

EDIT: I just find there's a __curPos akin to C's __FILE__/__LINE__ which has different meanings in different places. https://github.com/NixOS/nix/blob/master/doc/manual/src/release-notes/rl-1.7.md

@sternenseemann
Copy link
Member

sternenseemann commented May 14, 2023

I'd think these as unreliable implementation details.

Actually very reliable: Overloading -,* and / has been working ever since those operators were first introduced in 2013 (5d147e1, 4770167). Same goes for the comparison operators in the same year (3d77b28).

Ever since <…> became declarative in 2014 (62a6eeb, 3d221a7), overloading search path expression has been possible—how this can be leveraged in conjunction with scopedImport is even documented.

Additionally, behavior that is not documented is still known—if everything that is not documented about Nix were to be changed arbitrarily, a lot of actual expressions would break.

I would like to see Nix be documented as the pragmatic, 20 year old project that it is rather than have it changed according to some preconceived notion what it should have been first in order to be documented.

It's good to discourage undocumented identifiers starting with __ in any place

Sure, going forward. Still, a better practice is no reason to unnecessarily break stuff—this is all I want. We don't need to document and bless this behavior, but breaking it needs careful consideration and should be orthogonal to documenting Nix.

IRCC, identifiers starting with __ are considered language reserved

But is that documented…? :)

@oxalica
Copy link
Contributor

oxalica commented May 14, 2023

Actually very reliable: Overloading -,* and / has been working ever since those operators were first introduced in 2013 (5d147e1, 4770167). Same goes for the comparison operators in the same year (3d77b28).

Ever since <…> became declarative in 2014 (62a6eeb, 3d221a7), overloading search path expression has been possible—how this can be leveraged in conjunction with scopedImport is even documented.

Additionally, behavior that is not documented is still known—if everything that is not documented about Nix were to be changed arbitrarily, a lot of actual expressions would break.

I get you point about compatibility, but I don't really like this much python-ish dynamic, especially to overload buitlins, since it can hurt readability (for a bare file without context) and performance. It makes nix hard to cache the preprocessed source files. scopedImport is relatively rare but I did see some frameworks in wild leveraging them. Not a fan of this approach.

@sternenseemann
Copy link
Member

readability (for a bare file without context)

True, but this is due to scopedImport existing, not overloading. You can pass in a different builtins set just as well.

performance

Since overloading is only possible via non-dynamic scope, it can be implemented very efficiently—at least if you do a static scope analysis before dispatching which even C++ Nix can to an extent as far as I'm aware. You only need to have a function call if the overloading identifier has been re-defined in scope which can be known statically (or semi-statically with scopedImport).

@infinisil
Copy link
Member

True, but this is due to scopedImport existing, not overloading

I did a little dive into scopedImport in #8024 btw, which talks about these two problems, the confusing free variables problem for scopedImport, and the separate problem when combined with shadowing builtins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug documentation language The Nix expression language; parser, interpreter, primops, evaluation, etc
Projects
Status: No status
Development

No branches or pull requests

4 participants