crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.37k stars 1.62k forks source link

[RFC] Change inclusive/exclusive range operator #13675

Open Silentdoer opened 1 year ago

Silentdoer commented 1 year ago

should use .. and ..= (see https://github.com/crystal-lang/crystal/issues/13675#issuecomment-1640649067 below)

Blacksmoke16 commented 1 year ago

What's the use case/reasoning?

z64 commented 1 year ago

@Blacksmoke16 While it is "too late" to change it in Crystal, and I do not agree with changing it now - even for 2.x - I can explain my own frustrations with it:

Other languages have made the difference very distinct: .. is exclusive, and ..= is inclusive, the = being a strong visual marker that it "includes" the next index. Some that I use also go as far to not have .., but ..< and ..= instead.

Essentially: I couldn't tell you how often (every few months?) but some issue arises with this (prod bug or otherwise) and it wastes our time a bit. Crystal is the only language I make this mistake in.

beta-ziliani commented 1 year ago

@Silentdoer I took the liberty of editing your issue to make it more descriptive, hope you don't mind!

straight-shoota commented 1 year ago

Crystal is the only language I make this mistake in.

Ruby is the same (that's where Crystal inherited it from).

straight-shoota commented 1 year ago

It could be an option to consider ..< and ..= as alternative syntax (and eventual replacement). It would not break anything and be compatible with .. and ..., so should technically be possible.

z64 commented 1 year ago

The following is a patch that seems to work for those interested in carrying it forward / using it in their own code;

```diff From 855dfd29980252549401c4696959355193265248 Mon Sep 17 00:00:00 2001 From: Zac Nowicki Date: Tue, 18 Jul 2023 14:48:49 -0400 Subject: [PATCH] Add ..< for exclusive ranges and ..= for inclusive --- src/compiler/crystal/syntax/lexer.cr | 4 ++++ src/compiler/crystal/syntax/parser.cr | 17 +++++++++++++---- src/compiler/crystal/syntax/token.cr | 2 ++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/compiler/crystal/syntax/lexer.cr b/src/compiler/crystal/syntax/lexer.cr index 33f991c5f..d82c65c60 100644 --- a/src/compiler/crystal/syntax/lexer.cr +++ b/src/compiler/crystal/syntax/lexer.cr @@ -414,6 +414,10 @@ module Crystal case next_char when '.' next_char :OP_PERIOD_PERIOD_PERIOD + when '<' + next_char :OP_PERIOD_PERIOD_LT + when '=' + next_char :OP_PERIOD_PERIOD_EQ else @token.type = :OP_PERIOD_PERIOD end diff --git a/src/compiler/crystal/syntax/parser.cr b/src/compiler/crystal/syntax/parser.cr index 016d3d7e3..93b1ac4bb 100644 --- a/src/compiler/crystal/syntax/parser.cr +++ b/src/compiler/crystal/syntax/parser.cr @@ -511,7 +511,11 @@ module Crystal def parse_range location = @token.location - if @token.type.op_period_period? || @token.type.op_period_period_period? + case @token.type + when .op_period_period?, + .op_period_period_period?, + .op_period_period_lt?, + .op_period_period_eq? exp = Nop.new else exp = parse_or @@ -519,9 +523,11 @@ module Crystal while true case @token.type - when .op_period_period? + when .op_period_period?, + .op_period_period_eq? exp = new_range(exp, location, false) - when .op_period_period_period? + when .op_period_period_period?, + .op_period_period_lt? exp = new_range(exp, location, true) else return exp @@ -4674,7 +4680,10 @@ module Crystal if current_char.ascii_whitespace? return nil end - when .op_period_period?, .op_period_period_period? + when .op_period_period?, + .op_period_period_period?, + .op_period_period_lt?, + .op_period_period_eq? return nil unless allow_beginless_range else return nil diff --git a/src/compiler/crystal/syntax/token.cr b/src/compiler/crystal/syntax/token.cr index 125ec14ee..5ba4a29b5 100644 --- a/src/compiler/crystal/syntax/token.cr +++ b/src/compiler/crystal/syntax/token.cr @@ -158,6 +158,8 @@ module Crystal OP_PERIOD # . OP_PERIOD_PERIOD # .. OP_PERIOD_PERIOD_PERIOD # ... + OP_PERIOD_PERIOD_LT # ..< + OP_PERIOD_PERIOD_EQ # ..= OP_SLASH # / OP_SLASH_SLASH # // OP_SLASH_SLASH_EQ # //= -- 2.41.0 ``` ```cr a = [1, 2, 3] # starting index b = a[0..<2] c = a[0..=2] p(b) p(c) # begin-less b = a[..<2] c = a[..=2] p(b) p(c) ``` ``` [1, 2] [1, 2, 3] [1, 2] [1, 2, 3] ```
asterite commented 1 year ago

I can't remember who, but someone told me that between .. and ..., the extra dot pushes the last element away from the range, making the range exclude it.

Sija commented 1 year ago

IMO It looks like a highly subjective matter, neither option is objectively better than the other.

zw963 commented 1 year ago

Yes, the ruby usage of .. and ... not good, though, I realized this after contacting other languages, because it is easy to confuse, Ruby is my first deep learn programming language, i thought many language were use like this.

I propose add new operator ..= ..<, as described in https://github.com/crystal-lang/crystal/issues/13675#issuecomment-1640701917, it make sense, but never change/drop old behavior.

HertzDevil commented 1 year ago

One nice "feature" of the three-dot operator is that code like foo = ... or foo.bar(1, ..., 2) is perfectly valid syntax, so it won't throw up syntax highlighting in documentation where the dots don't even express a Range at all.

nobodywasishere commented 10 months ago

I think this change should be (re)considered, as IMO it's clearer which is which from a cursory glance, especially for those new to the language. I can create a PR with @z64's patch if it would be accepted.

beta-ziliani commented 10 months ago

I wouldn't oppose eventually to have something like straight-shoota said, but there're two issues to consider:

  1. We make them alternate syntax, and now we have two ways to say the same: this brings confusion to newcomers, and is one of the things that Crystal avoided in the first place; to have two ways of saying the same.
  2. We deprecate the old syntax, meaning all the current code will throw warnings like crazy. Also, I'm not sure if we already added deprecation at the level of the parser (I vaguely remember this being considered/implemented).

At this point I'm leaning towards adopting 2, but for 2.0.

zw963 commented 10 months ago

I never misunderstood .. with ..., and when code review, i will notice it specially, because i come from Ruby land.

Yes, we just inherited from ruby, there is no more thinking when design Crystal, but, that not so bad anyway, is Ruby is discussing replace it to use new syntax too?

Maybe, we need add some document,for users come other land, how to remember, sure, for me, it not necessary, because i familiar with it, but for the new guys, you can try remember like this:

for .., just a NORMAL range, that it, includes the end. for ..., the third dot occupied the position of the end, so, end is not included.

Clear enough?

z64 commented 10 months ago

I've been using Crystal since 2017 - I fully understand the difference, but for me it is just too visually subtle to not miss in reviews, with mistakes made both by myself and peers of varying level of Crystal experience. Sometimes it is just a typo, other times its a misunderstanding. Then I see some people do array[i..j -1] because they don't know ... exists.

In other languages that use different syntax (like ..< and ..=), I have never once made a mistake in the first place - there is nothing to "try to remember" in those languages, because you must pick one, and it is visually distinct.

(on top of the fact that .. is the opposite of other languages - they do not include the end, furthering the confusion when coming back to Crystal)

zw963 commented 10 months ago

(on top of the fact that .. is the opposite of other languages - they do not include the end, furthering the confusion when coming back to Crystal)

Yes, this is the point, the opposite of other languages, so we should consider add new syntax which compatible to other languages, but, leave old behavior works forever.

straight-shoota commented 10 months ago

(on top of the fact that .. is the opposite of other languages - they do not include the end, furthering the confusion when coming back to Crystal)

I'm surprised by this statement. In which languages does .. denote an open end range?

oprypin commented 10 months ago

Nim has the syntax 0 .. 9 and 0 ..< 10 (actually originally it was technically 0 .. <10, but not anymore). Adding this as an alias to ... would be fine by me. ..= is unnecessary, actually it's worse because it's an asymmetric operator denoting a symmetric operation. And my topmost preference for how to resolve this issue is: no action. Sometimes "no churn" is better than "slight improvement and huge churn".

z64 commented 10 months ago

(on top of the fact that .. is the opposite of other languages - they do not include the end, furthering the confusion when coming back to Crystal)

I'm surprised by this statement. In which languages does .. denote an open end range?

This is what I mean; the expression of array[..2] - or equivalent "subslice ending at index 2" - is the same in all except Crystal:

Examples Crystal: ``` ~ $ cr i Crystal interpreter 1.9.2 (2023-08-24). EXPERIMENTAL SOFTWARE: if you find a bug, please consider opening an issue in https://github.com/crystal-lang/crystal/issues/new/ icr:1> a = [1, 2, 3, 4] => [1, 2, 3] icr:2> a[..2] => [1, 2, 3] ``` The rest: ``` ~ $ python Python 3.11.5 (main, Sep 2 2023, 14:16:33) [GCC 13.2.1 20230801] on linux Type "help", "copyright", "credits" or "license" for more information. >>> a = [1,2,3] >>> a[:2] [1, 2] ``` ``` ~ $ cat main.rs fn main() { let a = [1, 2, 3, 4]; println!("{:?}", &a[..2]); } ~ $ ./main [1, 2] ``` ``` ~ $ cat main.odin package main import "core:fmt" main :: proc() { a := [3]int{1, 2, 3, 4} fmt.println(a[:2]) } $ odin run main.odin -file [1, 2] ``` ``` ~ $ cat main.zig const std = @import("std"); /* (sorry i've never used zig, maybe a nicer way to write this) */ pub fn main() void { var array = [_]i32{ 1, 2, 3, 4 }; for (array[0..2]) |elem| { std.debug.print("{}, ", .{elem}); } } ~ $ zig run main.zig 1, 2, ⏎ ``` Next ones I don't think they have a range/slice operator, but the equivalent functions: ``` ~ $ node Welcome to Node.js v20.7.0. Type ".help" for more information. > a = [1, 2, 3, 4] [ 1, 2, 3, 4 ] > a.slice(0,2) [ 1, 2 ] ``` ``` 1 [1] => 2 ) ```

Its "fine" if you exclusively work in Crystal, but after swapping between other langs for extended periods of time, I have to be careful that I remember the difference in Crystal's semantics / that I need to use the other ... if I want what I probably meant.

oprypin commented 10 months ago

@z64 That just seems like.. you could ban all usages of .. in your team and that's the problem solved.

straight-shoota commented 10 months ago

@z64 Python and Odin are not actually a .. operator, though (arguably, it's just rotated by 90°). They are specifically targeted for subslicing, so I'd think they're similar to the examples with JS and PHP slice methods. Apparently these languages prefer (or expect) open ended ranges for subslicing. The same applies to Zig actually, where .. is exclusively used for subslicing. I'm not sure if this is actually reasonable. Looking at the Crystal repo, subslicing operations (#[Range] method) much more often use closed end ranges than open ended ones. I haven't looked too deeply into the individual examples, but as a general take away, I'm glad to have the option of choosing either semantic depending on the specific use case.

I found this comparison across multiple languages: http://rigaux.org/language-study/syntax-across-languages/VrsDatTps.html#VrsDatTpsRng In all languages listed there, .. designates an inclusive range. Kotlin is another example of that category.

And I really think that makes the most sense for .., especially when there's operators for both open and closed ranges.

I think Rust and Zig made a very bad choice with .. meaning open end and ..=/...[^1] meaning closed end. They got it the wrong way around! This contradicts the intuitive wisdom of https://github.com/crystal-lang/crystal/issues/13675#issuecomment-1644016280 and deviates from the semantics for .. which apparently most other languages before them had established. So if you're confused about whether .. means inclusive or exclusive, you might just thank Rust and Zig for spoiling the simplicity 😛

I don't know what the reasons are for their syntax decisions. Maybe they have good reasons. But I can't really see any right now. IMO the opposite mapping would've been more intutive and congruent with previous works.

Interestingly, Zig has these two range syntaxes but they are used in completely different contexts: .. is for slices, ... for select branches. So they're not even directly related or interchangeable. Perhaps they just evolved independently somehow.

Anyway, I'm not saying that the semantics Crystal inherited from Ruby (.. meaning closed end and ... meaning open end) are the best solution. But as far as I can tell, it's a much better choice than the inverted semantics in Rust and Zig. Other solutions might be even better, though.

[^1]: Rust originally had ... for closed end range, but transitioned to ..= for clarity (https://github.com/rust-lang/rust/issues/28237). IMHO: Maybe they noticed that they made a bad choice originally but couldn't swap .. and ... anymore so they tried to make the best out of it.

zw963 commented 9 months ago
  1. Rust originally had ... for closed end range, but transitioned to ..= for clarity (https://github.com/rust-lang/rust/issues/28237). IMHO: Maybe they noticed that they made a bad choice originally but couldn't swap .. and ... anymore so they tried to make the best out of it.

Because i knew the reverse usage of .. and ... from rust, so i thought most of languages use this same syntax ... i guess i am wrong.