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] Change inclusive/exclusive range operator #13675

Open
Silentdoer opened this issue Jul 18, 2023 · 21 comments
Open

[RFC] Change inclusive/exclusive range operator #13675

Silentdoer opened this issue Jul 18, 2023 · 21 comments

Comments

@Silentdoer
Copy link

Silentdoer commented Jul 18, 2023

should use .. and ..= (see #13675 (comment) below)

@Blacksmoke16
Copy link
Member

What's the use case/reasoning?

@z64
Copy link
Contributor

z64 commented Jul 18, 2023

@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:

  • Inclusive/Exclusive range syntax in crystal is the opposite of each other language that I use (.. is exclusive in others)
  • .. and ... is a very subtle difference visually; I often tend to miss the improper use of one or the other in code review, or I simply typo one for the other.

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 beta-ziliani changed the title The range operator needs to be changed [RFC] Change inclusive/exclusive range operator Jul 18, 2023
@beta-ziliani
Copy link
Member

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

@straight-shoota
Copy link
Member

Crystal is the only language I make this mistake in.

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

@straight-shoota
Copy link
Member

straight-shoota commented Jul 18, 2023

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
Copy link
Contributor

z64 commented Jul 18, 2023

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

From 855dfd29980252549401c4696959355193265248 Mon Sep 17 00:00:00 2001
From: Zac Nowicki <zachnowicki@gmail.com>
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
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
Copy link
Member

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
Copy link
Contributor

Sija commented Jul 20, 2023

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

@zw963
Copy link
Contributor

zw963 commented Jul 23, 2023

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 #13675 (comment), it make sense, but never change/drop old behavior.

@HertzDevil
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

zw963 commented Dec 8, 2023

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
Copy link
Contributor

z64 commented Dec 9, 2023

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
Copy link
Contributor

zw963 commented Dec 9, 2023

(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
Copy link
Member

(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
Copy link
Member

oprypin commented Dec 9, 2023

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
Copy link
Contributor

z64 commented Dec 9, 2023

(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 ]
<?php
$array = array(1, 2, 3, 4);
$subArray = array_slice($array, 0, 2);
print_r($subArray);

Array
(
    [0] => 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
Copy link
Member

oprypin commented Dec 9, 2023

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

@straight-shoota
Copy link
Member

straight-shoota commented Dec 9, 2023

@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 #13675 (comment) 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.

Footnotes

  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
Copy link
Contributor

zw963 commented Dec 11, 2023

  1. Rust originally had ... for closed end range, but transitioned to ..= for clarity (Tracking issue for ..= inclusive ranges (RFC #1192) -- originally ... rust-lang/rust#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests