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

RFC: Floor division operator #832

Merged
merged 6 commits into from
Mar 20, 2023
Merged

Conversation

petrihakkinen
Copy link
Contributor

@petrihakkinen
Copy link
Contributor Author

I'm sorry, I couldn't find a way to add the "rfc" label to this PR before or after creating it :/

@alexmccord alexmccord added the rfc Language change proposal label Feb 8, 2023
@petrihakkinen
Copy link
Contributor Author

Thanks Alexander!

@petrihakkinen
Copy link
Contributor Author

New revision: minor styling tweaks, added //=

@zeux
Copy link
Collaborator

zeux commented Feb 28, 2023

Anecdotal stats based on Roblox internal codebase (~1-2 MLOC, ~10-20M bytecode instructions):

  • 582 calls to math.floor
  • out of these, 200 calls are emulating integer division directly (so a little over 1/3rd)

Obviously our codebase is a bit biased in that it doesn't do a whole lot of integer math directly.

There could also be some divisions that assume divisibility and forget/omit rounding; the code base has a grand total of 4567 division operators, so some percentage of these might be bugs, or the value might be implicitly rounded later (eg when passed to a C API).

@zeux
Copy link
Collaborator

zeux commented Feb 28, 2023

Something else that might be worth mentioning is precedents around this in other languages (except for Lua 5.3 which obviously supports this as proposed).

  • Python has // operator with the same semantics; it's particularly helpful there as it avoids a common footgun of 5 / 3 == 1 despite having integer types.
  • JS/TS don't have a // operator, despite working with the similar, floating-point-only, environment, and require Math.floor instead
  • PHP doesn't have a // operator but it does have an intdiv function which is equivalent to // except that it truncates (doesn't floor)
  • R has support for integer division operator, although they call it %/% (whoah)
  • Julia has an integer division operator, that uses a Unicode symbol ÷ (whoah)
  • Most statically typed languages don't have this distinction, instead relying on types; often they adopt C rules for the division (rounding to 0, instead of rounding to negative infinity), however some newer languages like Go fix this and round down, as does this proposal.
  • Zig is a notable exception as it omits integer division operator entirely, instead requiring explicit use of @divTrunc/@divFloor functions

@petrihakkinen
Copy link
Contributor Author

Thanks, that's a nice list! I also did some research on other languages (not as extensive as yours), but decided not to have it in the RFC because "it's supported by language X" is sometimes not a good selling point :) But you're right that someone coming from those languages would be more at home if Luau also had this operator. Should I add these to the RFC?

@zeux
Copy link
Collaborator

zeux commented Mar 15, 2023

Should I add these to the RFC?

I don't think we need this exhaustive list, but maybe just mention that the feature has precedents in other languages (listed).

One other thing I was thinking of in regards to this proposal is that it can help us clean up the relationship between primitive operators for % - not operationally (this would not affect the implementation in any way), but from the documented semantics perspective. Namely, right now Lua [5.1] defines % as a - floor(a / b) * b for numbers, but it doesn't suggest what floor means/should mean for different types, and it doesn't give a way to access floor for a type that is not a number. If we had //, we could define % in terms of // which may be helpful for documentation purposes, and if a custom type implemented % and //, it would make the behavior of % more obvious. Of course in some sense this just moves the question "what is floor?" from the definition of % to the definition of // :) But I do think it helps this operator make a little bit more sense vs a world without %...

@petrihakkinen
Copy link
Contributor Author

@zeux Good points, thanks! Agreed about it clarifying the semantics. I've tweaked the RFC and added Python, R and Lua as examples of other languages having floor division operator.

@zeux zeux merged commit 0d6aacf into luau-lang:master Mar 20, 2023
polychromatist added a commit to polychromatist/tree-sitter-luau that referenced this pull request Apr 3, 2023
@vegorov-rbx
Copy link
Collaborator

We created a project internally to get this RFC scheduled and implemented.

@petrihakkinen
Copy link
Contributor Author

We created a project internally to get this RFC scheduled and implemented.

Great news, thanks!

@vegorov-rbx
Copy link
Collaborator

vegorov-rbx commented Aug 24, 2023

Just letting you know that this RFC is being implemented and feature might be available next week.

AmaranthineCodices added a commit that referenced this pull request Sep 1, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
- Updated Roblox copyright to 2023
- Floor division operator `//` (implements #832)
- Autocomplete now shows `end` within `do` blocks
- Restore BraceType when using `Lexer::lookahead` (fixes #1019)

# New typechecker

- Subtyping tests between metatables and tables
- Subtyping tests between string singletons and tables
- Subtyping tests for class types

# Native codegen

- Fixed macOS test failure (wrong spill restore offset)
- Fixed clobbering of non-volatile xmm registers on Windows
- Fixed wrong storage location of SSA reg spills
- Implemented A64 support for add/sub extended
- Eliminated zextReg from A64 lowering
- Remove identical table slot lookups
- Propagate values from predecessor into the linear block
- Disabled reuse slot optimization
- Keep `LuaNode::val` check for nil when optimizing `CHECK_SLOT_MATCH`
- Implemented IR translation of `table.insert` builtin
- Fixed mmap error handling on macOS/Linux

# Tooling

- Used `|` as a column separator instead of `+` in `bench.py`
- Added a `table.sort` micro-benchmark
- Switched `libprotobuf-mutator` to a less problematic version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc Language change proposal
Development

Successfully merging this pull request may close these issues.

None yet

4 participants