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

libexpr: add modulo operator #12617

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

libexpr: add modulo operator #12617

wants to merge 1 commit into from

Conversation

imnotpoz
Copy link

@imnotpoz imnotpoz commented Mar 7, 2025

Closes: #12616

Motivation

I want the modulo operator in nix

Context

first contribution to nix so please make sure to check that I didn't miss anything

I grepped the project for div and replicated everything I found but for mod so I'm pretty sure I got it all

works on my machine™ so hopefully it should be good

this uses the same modulo behaviour as in C++ (since it's implemented using % in C++), if any change in its behaviour is desired please let me know

this will error out with modulo by zero if the right hand side is 0, otherwise it can't fail

it's only implemented for integers because I haven't seen anyone use modulo with floats for anything I don't know if it'd be useful to anyone


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@sioodmy
Copy link

sioodmy commented Mar 8, 2025

we need modulo operator in nix !!!

@imnotpoz
Copy link
Author

imnotpoz commented Mar 8, 2025

fixed a typo in one of the error trace tests

@fricklerhandwerk fricklerhandwerk added the language The Nix expression language; parser, interpreter, primops, evaluation, etc label Mar 8, 2025
@roberth
Copy link
Member

roberth commented Mar 8, 2025

A mod function is definitely a good idea, and assigning % is probably ok despite the opportunity cost.
We could still use :: for future type annotations, and probably angle brackets and | for other metadata, as in #2511 if that idea gets a reboot.

@esther-ff
Copy link

May i ask what kind of opportunity cost is paid here for using % as modulo? You can say you even gain something (universality) because most languages use % as their symbol for modulo.

@piegamesde
Copy link
Member

Some thoughts in random order:

  • Please add a proper AST node to it instead of desugaring to __mod. You are probably just copying how the other operators work, but actually that is a design mistake which should not be repeated for future additions.
  • Please check how other languages implement modulo semantics and if the C++ ones are desirable here. Unfortunately, C++ is not that well designed and it would be unfortunate to inherit its design mistakes out of mere inertia. See https://trofi.github.io/posts/330-another-nix-language-nondeterminism-example.html as an extreme example of this
  • Nix is a DSL, and its usage in numerical applications is rather low. I am not fully convinced that modulo operations are sufficiently common to warrant their own operator in the syntax. (Personally, I'd rather try to go down the path of having first-class infix functions and custom definable quasi-operators.) Either way, the best way to proceed is to add this under an experimental feature flag and then do an evaluation on usage patterns like in [RFC 0148] Pipe operator rfcs#148

@roberth
Copy link
Member

roberth commented Mar 8, 2025

It's a bit of a question about unknown unknowns, but ideas like #2511 make me think that it would be good to use operators only for often-used things that are relevant for a configuration language.
Nix isn't really about numerical computations.

Having chatted with the friendly Lix folks, they're a bit more hesitant about assigning %. I don't think that has to be definitive, but they've brought up a good argument that this could introduce compatibility problems when this gets used in the wild while some users haven't upgraded to recent "versions" of the language implementations yet.

If we were to treat this as the optimized version of Nixpkgs lib.mod, we don't run that risk.
% could perhaps be added later when we have better approaches for dealing with this kind of change in the ecosystem.

Could you remove the operator part and keep the mod function?

@imnotpoz
Copy link
Author

imnotpoz commented Mar 8, 2025

Some thoughts in random order:

* Please add a proper AST node to it instead of desugaring to `__mod`. You are probably just copying how the other operators work, but actually that is a design mistake which should not be repeated for future additions.

I did copy div specifically - if I were to do it the proper way I'd probably want to fix the other operators as well and that's not in scope of this PR

either way I'm not sure how long it would take me to fix all of them but I'm willing to try

* Please check how other languages implement modulo semantics and if the C++ ones are desirable here. Unfortunately, C++ is not that well designed and it would be unfortunate to inherit its design mistakes out of mere inertia. See https://trofi.github.io/posts/330-another-nix-language-nondeterminism-example.html as an extreme example of this

I am pretty sure that modulo and division behaviour should match - Nix implements division using the C++ / operator so to match that I'm using C++'s %

if I were to make modulo do something else it wouldn't match with division and I feel like that's the biggest concern

* Nix is a DSL, and its usage in numerical applications is rather low. I am not fully convinced that modulo operations are sufficiently common to warrant their own operator in the syntax. (Personally, I'd rather try to go down the path of having first-class infix functions and custom definable quasi-operators.) Either way, the best way to proceed is to add this under an experimental feature flag and then do an evaluation on usage patterns like in [[RFC 0148] Pipe operator rfcs#148](https://github.com/NixOS/rfcs/pull/148)

I've had to implement a mod function twice already which obviously doesn't mean that it'll be commonly used or not but it seems like such a basic math operation that's also very easy to implement properly (literally just a passthrough to C++'s % - the tests all pass), with close to zero maintenance cost that I don't see why you'd want to keep it experimental

@piegamesde
Copy link
Member

I did copy div specifically - if I were to do it the proper way I'd probably want to fix the other operators as well and that's not in scope of this PR

either way I'm not sure how long it would take me to fix all of them but I'm willing to try

Nope, don't bother fixing them. Having to maintain backwards compatibility with the unintended semantic consequences is a pain, and given these having a proper dedicated operator does not help much. (In case you don't know, the specific issue is that 2 * 4 desugars to (__mul 2 4) which means that one can override the meaning of * by shadowing __mul. This cannot be changed for backwards compatibility, but at the very least we want to guarantee that newly added operators are not overridable in the same way.)

@imnotpoz
Copy link
Author

imnotpoz commented Mar 8, 2025

It's a bit of a question about unknown unknowns, but ideas like #2511 make me think that it would be good to use operators only for often-used things that are relevant for a configuration language. Nix isn't really about numerical computations.

Having chatted with the friendly Lix folks, they're a bit more hesitant about assigning %. I don't think that has to be definitive, but they've brought up a good argument that this could introduce compatibility problems when this gets used in the wild while some users haven't upgraded to recent "versions" of the language implementations yet.

If we were to treat this as the optimized version of Nixpkgs lib.mod, we don't run that risk. % could perhaps be added later when we have better approaches for dealing with this kind of change in the ecosystem.

Could you remove the operator part and keep the mod function?

regarding the last part of my message above, this is the only real issue I see with this but I also don't necessarily agree

% is almost universally recognized as the modulo operator among most languages and to me keeping it free for another possible thing in the future sounds like being different for the sake of being different - I might not know about something that would make you hesitant to use % for modulo though

I'd personally keep the operator but I get your worries, I'll remove it

@imnotpoz
Copy link
Author

imnotpoz commented Mar 8, 2025

Nope, don't bother fixing them. Having to maintain backwards compatibility with the unintended semantic consequences is a pain, and given these having a proper dedicated operator does not help much. (In case you don't know, the specific issue is that 2 * 4 desugars to (__mul 2 4) which means that one can override the meaning of * by shadowing __mul. This cannot be changed for backwards compatibility, but at the very least we want to guarantee that newly added operators are not overridable in the same way.)

ah, that makes sense thank you

if I'm getting rid of the operator, should I still somehow get rid of __mod or is that not necessary anymore?

@TeofilC
Copy link

TeofilC commented Mar 9, 2025

https://en.m.wikipedia.org/wiki/Modulo#In_programming_languages is a helpful list for seeing how different languages implement the modulo operator.

@imnotpoz
Copy link
Author

imnotpoz commented Mar 9, 2025

again, I'm not sure we want any other behaviour than the one that matches the division operator

and I don't think changing division behaviour is very backwards compatible :P

@piegamesde
Copy link
Member

again, I'm not sure we want any other behaviour than the one that matches the division operator

I don't understand this sentence on a technical level. Integer division, unlike modulo, is pretty much uniquely defined. Therefore, I don't see how one would need to be "compatible" with the division operator. Modulo is a function defined w.r.t. a division operation, sure, and that division operation is fixed, but this doesn't change the fact that there are multiple possible ways of defining modulo in the first place. For that matter, many languages offer multiple definitions as separate library functions.

Looking at the semantics in detail (thanks to that great Wikipedia article), I do very much prefer floored or euclidian semantics over truncated (as used by C++), because it guarantees a positive result. With truncated semantics, most of the time I do ((a % b) + b) % b (don't quote me on the exact formula) to get the semantics I actually want.

@imnotpoz
Copy link
Author

imnotpoz commented Mar 9, 2025

a / b in integer division will give us some quotient and leaves out a remainder - we want a % b to be that remainder, in other words a = b * (a / b) + (a % b) (assuming all integer operations of course)

e.g. the example from the tests:

5 / 2 = 2 => 5 = 2 * 2 + R => R = 1
5 / (-2) = (-2) => 5 = (-2) * (-2) + R => R = 1
(-5) / 2 = (-2) => (-5) = 2 * (-2) + R => R = -1
(-5) / (-2) = 2 => (-5) = (-2) * 2 + R => R = -1

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

Successfully merging this pull request may close these issues.

Modulo operator
7 participants