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

KCL: % is not an operator #4041

Open
adamchalmers opened this issue Sep 30, 2024 · 1 comment
Open

KCL: % is not an operator #4041

adamchalmers opened this issue Sep 30, 2024 · 1 comment
Labels
bug Something isn't working kcl Language and compiler features

Comments

@adamchalmers
Copy link
Collaborator

adamchalmers commented Sep 30, 2024

In many languages % is the modulo operator, but not in KCL! In KCL, % is used as a substitution in a pipeline (|>).

Trying to use % as an operator should be a parse error, or at the very least, a runtime error of some kind.

Unfortunately right now it's silently accepted, and for example this program:

let n = 10
let d = n % 2

evaluates d to 0.0 -- very bad.

I've reproduced this bug in a PR (#4042) but haven't yet fixed it.

@adamchalmers adamchalmers added bug Something isn't working coredump Issues that contain a coredump. kcl Language and compiler features and removed coredump Issues that contain a coredump. labels Sep 30, 2024
@jtran
Copy link
Collaborator

jtran commented Oct 28, 2024

I think it's fine to remove % as an operator. #4227 tries to change it to infix mod. Copying what I said from there:

My opinion is that having infix mod is inconsistent with the rest of the language. The way we divide and the way we do modulus should go together. There are different flavors, and as long as they match, it makes sense. Division as we've defined it matches with remainder. We have rem() for that. I think we should make mod() be a stdlib function similar to rem(), but that's implemented with Rust's rem_euclid.

@jessfraz jessfraz added this to the v1 Modeling App Launch milestone Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working kcl Language and compiler features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants