JuliaLang / JuliaSyntax.jl

The Julia compiler frontend
Other
266 stars 32 forks source link

Add modular operators +% -% >>% etc. #408

Open jakobnissen opened 5 months ago

jakobnissen commented 5 months ago

For the last ten years, there have been idle discussion about allowing a way for users to opt-in for explicitly overflowing integer operations. Julia's arithmetic operations do overflow by default, but it may in the future be possible to run Julia in a mode where overflows are detected, for testing purposes. If this is to be done, then there needs to be a way to signal that the user explicitly wants overflow, which is important in some low-level operations, such as hashing. For bitshifting, Julias definition of e.g. >> checks for overflow, which leads to suboptimal code. Analogous to the opt-in overflowing arithmetic ops like +%, users ought to be able to opt-in to the more efficient overflowing shifts, which in situations where bitshifts are a performance bottleneck.

This impements the JuliaSyntax side of Keno's https://github.com/JuliaLang/julia/pull/50790 (cc @Keno).

Note for reviewers:

  1. The tests currently fail, due to the test that the operators in JuliaSyntax matches Base.isoperator (presumably this is the flisp parser)? This is not a bug, this just means the flisp parser needs an update, too
  2. Keno has already updated the flisp parser to accept e.g. +% in https://github.com/JuliaLang/julia/pull/50790. However, in this PR I also add overflowing bitshifts to address https://github.com/JuliaLang/julia/issues/50225 (also see its linked issues). These overflowing bitshift operators also needs support in the flisp parser. I can give a try to updating the flisp parser based on Keno's PR. The overflowing bitshifts needs support in Julia, too, but the implementation should be straightforward. I'm willing to amend https://github.com/JuliaLang/julia/pull/50790 to implement, document and test these overflowing shifts, if @Keno gives his blessings.

This is my first PR to JuliaSyntax, and I don't really understand how JuliaSyntax is vendored into Julia, so if I need to do anything special for this PR, please let me know.

savq commented 4 months ago

Is there any reason this couldn't be done with operator suffixes like +ₒ, -ₒ, >>ₒ?

(I ask because Julia doesn't seem to mind using unicode for pretty conventional functions like xor.)

PallHaraldsson commented 4 months ago

[EDIT: I'm not downvoting the code, I don't feel like we need new operators agree with @c42f and I guess the code is good, elsewhere for a functional-form.]

It could be but shouldn't(?) xor is the only exception to using Unicode operator only, and that's because xor in the function form is ASCII. We have a policy of having ASCII only or both, and I at least do not like operators on Unicode only or both (plus possibly function form, that's not proposed here).

[xor is rare in most code, why Unicode operator, the standard one, for xor, was considered justified, and those you proposed are not, only obvious when explained, and even then unclear how to type. While with % that's not a problem, and it's understood as modular by many.]

PallHaraldsson commented 4 months ago

Please don't add new operators, until we know needed, and better solutions not possible.

E.g. I'm thinking maybe we rather need to solve it with the type system. It seems modular arithmetic is most useful for Unsigned types, e.g. have MInt128 down to MInt8 corresponding to UInts. Actually (down to) at least MInt6, and even MInt3, corresponding to my idea here:

https://github.com/JuliaLang/julia/pull/52828#issuecomment-1961137569

If we were ever to change the default types, e.g. Int[64], then those signed ones could be changed, and I'm actually unsure if anyone wants or needs them modular, except for the speed argument. The unsigned ones however need to be sometimes modular, but that's also more dangerous, but also not the default types, so we get away with it more. Maybe in the end we could just change the definition of the signed ones, not need 3 or 4 classes, i.e. adding MInt types...

c42f commented 3 months ago

The code looks pretty good here :+1:

I have misgivings about special syntax for >>%... along the lines of https://github.com/JuliaLang/julia/pull/52828#issuecomment-1989730820. But regardless of that, we should have any debate about actual syntax changes upstream in JuliaLang/Julia - not that many people are paying attention to the JuliaSyntax.jl repo.

Once something is decided upstream, we can enact it here! The procedure to get it into Base is to bump https://github.com/JuliaLang/julia/blob/master/deps/JuliaSyntax.version and the new version should get vendored in there.

c42f commented 3 months ago

I've added my thoughts about the alternatives to special syntax here: https://github.com/JuliaLang/julia/pull/50790#issuecomment-1989812730