rust icon indicating copy to clipboard operation
rust copied to clipboard

Tracking issue for exclusive range patterns

Open oli-obk opened this issue 9 years ago • 98 comments

Concerns

  • ~~Conflict with the syntax of slice patterns https://github.com/rust-lang/rust/issues/23121~~
    • https://github.com/rust-lang/rust/issues/62254#issuecomment-1700592538
  • General concern about the syntax of inclusive/exclusive patterns

History and status

oli-obk avatar Nov 18 '16 11:11 oli-obk

EDIT: the following is no longer true 12 jan 2019 nightly debug 2018:

Missing warning: unreachable pattern in the case when 1...10 then 1..10 are present in that order. That warning is present in case of 1..12 then 1..11 for example.

Example (playground link):

#![feature(exclusive_range_pattern)]
fn main() {
    let i = 9; //this hits whichever branch is first in the match without warning
    match i {
        1...10 => println!("hit inclusive"),
        1..10 => println!("hit exclusive"),//FIXME: there's no warning here
        // ^ can shuffle the above two, still no warnings.
        1...10 => println!("hit inclusive2"),//this does get warning: unreachable pattern
        1..10 => println!("hit exclusive2"),//this also gets warning: unreachable pattern

        _ => (),
    };

    //expecting a warning like in this:
    match i {
        1..12 => println!("hit 12"),
        1..11 => println!("hit 11"),//warning: unreachable pattern
        _ => (),
    }
}

ghost avatar Jul 30 '17 23:07 ghost

I've just been saved for the second time by the compiler when I did 1 .. 10, but meant 1 ..= 10. It might be worth considering that the compiler won't catch this error if exclusive ranges are allowed.

richard-uk1 avatar Jun 05 '18 12:06 richard-uk1

seems like exclusive range match would help to improve consistency ... why can i do range in a loop but not in a pattern?

 const MAXN:usize = 14;
 ... 
   for x in 0..MAXN  { blahblahblah(); }
 ...
   let s = match n {
      0..MAXN=>borkbork(),
      MAXN=>lastone(),
      _=>urgh(),
   };

also note that the following is not possible currently because the -1 will break the compile with "expected one of ::, =>, if, or | here"

   let s = match n {
      0..=MAXN-1=>borkbork(),
      MAXN=>lastone(),
      _=>urgh(),
   };

thank you

donbright avatar Jan 12 '19 22:01 donbright

@donbright I know its a poor consolation, but a good workaround which ensures backwards compatibility might be:

        match i {
            0..=MAXN if i < MAXN => println!("Exclusive: {}", i),
            MAXN => println!("Limit: {}", i),
            _ => println!("Out of range: {}", i)
        }

playground

Figured this suggestion might help some poor soul who came here and also battled with not being allowed to use 0..=MAXN-1

kentfredric avatar Jun 26 '19 02:06 kentfredric

It doesn't sound like much is going to happen on this issue, but (FWIW) I would argue against its inclusion in stable Rust: exclusive ranges on pattern matching makes it very easy to introduce bugs, as exclusive ranges are very rarely what the programmer wants in a pattern match (emphasis on match!). In these cases, I would argue that having the programmer be explicit is helpful; there is no reason to offer syntactic sugar in this case. (I landed here because of finding several bugs in a project that had turned this feature on; see https://github.com/tock/tock/issues/1544 for details.)

bcantrill avatar Jan 16 '20 18:01 bcantrill

To consistent with for and other cases,

  • either we have to support both inclusive and exclusive patterns in match.
  • or either we should deprecate exclusive patterns in both places.
// 0 to 10 (10 exclusive)
for a in 0..10 { 
  println!("Current value : {}", a);
}

// 0 to 10 (10 inclusive)
for a in 0..=10 { 
  println!("Current value : {}", a);
}
let tshirt_width = 20;
let tshirt_size = match tshirt_width {
    16 => "S",       // 16
    17 | 18 => "M",  // 17 or 18
    19..21 => "L",   // 19 to 21; 21 excusive (19,20)
    21..=24 => "xL", // 21 to 24; 24 inclusive (21,22,23,24)
    25 => "XXL",
    _ => "Not Available",
};

While we having inconsistencies like this even in smallest things, it's more harder for newcomers to learn and master Rust.

dumindu avatar May 19 '20 00:05 dumindu

ranges are very rarely what the programmer wants in a pattern match (emphasis on match!)

Arguments like that IMO are better grounds for lints, not "you can't have this".

Otherwise I'd be arguing you shouldn't be allowed to have a thing.is_file(), because it sometimes isn't what you expect: Usually you just want a readable thing that isn't a directory, and things like pipes and symbolic links and device nodes are acceptable, even if weird... but .is_file() will likely exclude those options for you. ( Hence why I proposed a lint for that )

And lets not even get into the rabbit warren of "stating files is bad, because you could have the underlying file change between the stat time and the time you modify it, use proper error handling instead" argument, because while I agree in part, its a decision you should make after understanding a lint telling you "why this is probably bad" and you decide whether or not it matters for your code.

kentfredric avatar May 28 '20 14:05 kentfredric

If I may add a datapoint. I tried to use an exclusive range today and the compiler error took me here. Turned out I wanted an inclusive range, so I for one am greatful that rust does not (stabley) allow exclusive ranges and I would argue against stablising as it would have caused me to shoot myself in the foot. :)

harrysarson avatar Aug 14 '20 16:08 harrysarson

I wanted exclusive ranges today, for a case where I had a variable that needed to be handled differently depending on which of the following ranges it fell into: 0..10, 10..100, 100..1000 and 1000..10000. In my opinion the code would have been cleaner if exclusive ranges were supported.

avl avatar Aug 20 '20 20:08 avl

for a case where I had a variable that needed to be handled differently depending on which of the following ranges it fell into: 0..10, 10..100, 100..1000 and 1000..10000.

You could just do a match on x.log10().floor()

phaux avatar Oct 07 '20 20:10 phaux

for a case where I had a variable that needed to be handled differently depending on which of the following ranges it fell into: 0..10, 10..100, 100..1000 and 1000..10000.

You could just do a match on x.log10().floor()

Yeah, good point! But at least to me, it wouldn't be as readable. The match would be something like:

0 => /* handle  case 0..10 */,
1 => /* handle case 10..100 */,
2 => /* handle case 100..1000 */,

Which isn't bad, but without the comment it wouldn't be immediately obvious that the case "0" handled the range 0..10 .

avl avatar Oct 07 '20 20:10 avl

It is not clear that

0..=9
10..=99
100..=999
1000..=9999

is less desirable.

However I would like to voice that I prefer having the option to have an exclusive range pattern for the sole case of the "half-open range" (#67264) that goes from a certain value unto the natural end of that range

workingjubilee avatar Oct 21 '20 23:10 workingjubilee

@workingjubilee I agree that the difference in readability is not entirely obvious.

But let's say we had these ranges instead:

# Constants:
const NOMINAL_LOW:i32 = 0;
const NOMINAL_HIGH:i32 = 100;
const OVERVOLTAGE:i32 = 200;
# Ranges:
NOMINAL_LOW .. NOMINAL_HIGH,
NOMINAL_HIGH .. OVERVOLTAGE,

In this case, having to write NOMINAL_LOW..=NOMINAL_HIGH-1 seems slightly worse. than NOMINAL_LOW..NOMINAL_HIGH .

I know matching on floats is being phased out, but if that capability was intended to be kept, using inclusive ranges would have been quite difficult, since you couldn't just decrement by 1 to get the predecessor of NOMINAL_HIGH. Will rust ever support matching on user types? Like on a fixed point type, big number, or something like that?

avl avatar Oct 22 '20 06:10 avl

How we can push this issue/fix forward?

dumindu avatar Oct 22 '20 12:10 dumindu

I would prefer this to exist for consistency, and because I personally just find half-open ranges easier to reason about, even in match statements.

I believe many of the concerns listed could be addressed via the existing exhausiveness checking, and a clippy lint against using _ in combination with range patterns when an exhaustive pattern can be easily specified.

For example, instead of:

match x {
    0..9 => foo(),
    _ => bar(),
}

The lint could push you towards:

match x {
    0..9 => foo(),
    9.. => bar(), // Would fail to compile if you tried `10..` due to exhaustiveness checking
}

At which point it would be very obvious that perhaps you meant:

match x {
    0..10 => foo(),
    10.. => bar(),
}

Diggsey avatar Oct 28 '20 16:10 Diggsey

That seems like a good idea, and I would actually say this almost shouldn't be a Clippy lint because it's fundamental to Rust's exhaustiveness capabilities, but it could be at least tested in Clippy.

I suppose there's a risk of false positives or arguments on style, and it making rustc too opinionated. Exclusive and half-open range patterns definitely make more sense together rather than separately.

workingjubilee avatar Oct 28 '20 19:10 workingjubilee

@Diggsey, I think that that's a great suggestion, though I would actually go a step further and have the compiler itself warn on non-exhaustive patterns (that is, use of _ when using exclusive range patterns) -- I think that one would much rather explicitly turn off the warning to allow any reader of the code to see what's going on. I know that my comment was very controversial, but I would also add that it was effectively coming from the field: it should be an important data point that a project enabled this feature, and it was rampantly misused, introducing (many) subtle bugs. Coming from C, I would liken exclusive range patterns to switch fallthrough: yes, there are cases where it's useful -- but it's so often done accidentally that the programmer is also required to mark it as /*FALLTHROUGH*/ to prevent lint from warning on it. One of Rust's great strengths is that it (generally) doesn't have trap doors like this; forcing the programmer to either use exhaustive pattern matching (which would have caught the bugs that I hit, for whatever it's worth) or explicitly turn off the warning seems like an excellent compromise!

bcantrill avatar Oct 28 '20 20:10 bcantrill

"RangeFrom" patterns (X..) will be partially stabilized in #67264. No decision was reached regarding "RangeTo" (..Y) or "Range" patterns (X..Y).

workingjubilee avatar Nov 03 '20 00:11 workingjubilee

Nearly four years, and it's still experimental. Sigh. Both inclusive and exclusive ranges are useful; when working with matches for odd usages of ASCII-like bytes (current: working on parsing the mess that is the DNS NAPTR service field), both inclusive and exclusive ranges may make code far more readable in different circumstances. Given that constant maths isn't possible in range patterns yet, it's a royal pain to make code clearly demonstrate its purpose with one arm tied behind your back. I'm all for a compiler and linter making it difficult to make mistakes, but, ultimately, good software requires comprehensive testing practices to go with it.

Good software needs to read like a good book; any choice about syntax, naming and the like should always be driven from that observation.

raphaelcohn avatar Nov 09 '20 14:11 raphaelcohn

Just to weigh in as a brand new user of the language — I agree with @raphaelcohn that both inclusive and exclusive ranges have their use cases, I really don't understand the logic for locking this behind an "experimental" error. IMO, this type of "Oh, that syntax is probably not what you meant" nannying should be the purview of a linter, not a compiler. (Take that with a grain of salt since, again, I'm brand new to the language.)

The context is that I'm running through a Udemy crash course and completing one of the exercises:

// 2. For each coord in arrow_coords:
//
//   A. Call `coord.print_description()`
//   B. Append the correct variant of `Shot` to the `shots` vector depending on the value of
//   `coord.distance_from_center()`
//      - Less than 1.0 -- `Shot::Bullseye`
//      - Between 1.0 and 5.0 -- `Shot::Hit(value)`
//      - Greater than 5.0 -- `Shot::Miss`
for coord in arrow_coords {
    coord.print_description();
    let dist = coord.distance_from_center();
    shots.push(match dist {
        0.0..1.0 => Shot::Bullseye,
        1.0..5.0 => Shot::Hit(dist),
        _ => Shot::Miss,
    });
}

I sat back in my chair grinning smugly at my elegant solution for a few seconds, but the compiler quickly threw cold water on my enthusiasm. 😛 And now I have to go Googling for how to configure the compiler to let me do this. I know that I could use an if/else expression to achieve the same result, but IMO it would not be as readable.

dannymcgee avatar Dec 13 '20 23:12 dannymcgee

@dannymcgee The Rust compiler comes with a built-in linter. The lints built into the Rust compiler have a very low rate of false positives by design, but it is not non-zero. Nonetheless, having extremely strong error messages and linting facilities in the compiler by default is a part of the language.

You will wish to use rustup to install the nightly version of the compiler (rustup install nightly) and then enable the feature using

#![feature(exclusive_range_pattern)]

at the top of the file.

However, I should note that matching on floats is problematic since they are less amenable to what Rust is doing with match, so you may run into challenges anyways.

workingjubilee avatar Dec 15 '20 02:12 workingjubilee

#83918 was opened with a proposal to stabilize the X.. pattern format and has begun its final comment period. This does not include the X..Y and ..Y patterns.

workingjubilee avatar May 04 '21 20:05 workingjubilee

The specific cases of ranges in slice patterns will not be stabilized in #83918, per the discussion in that PR and the explanatory comment here: https://github.com/rust-lang/rust/issues/67264#issuecomment-854921128 The interaction of range patterns inside slice patterns should be RFCed.

workingjubilee avatar Jun 04 '21 18:06 workingjubilee

Experience report, seconding the early comments in this issue: This being experimental just stopped me from writing the following bug:

b'a'..b'z' | b'A'..b'Z' | b'0'..b'9' => {

riking avatar Jul 18 '22 23:07 riking

With a steadier grasp on the language, I can now add:

The for x in 5..10 style of cases that was mentioned by a few programmers a while back features the range 5..10 as an expression, not a pattern. I believe there is an unfortunate amount of confusion in that regard due to other choices, but that asserting the expression and pattern syntax should be made the same due to that superficial similarity is not ideal. I also think that the multiple experience reports highlight an important detail:

What is ergonomic for integer, floating point, and character ranges are not the same. Yet in Rust they all go through the same mechanism here.

workingjubilee avatar Jul 19 '22 07:07 workingjubilee

I just had an idea for that: what if we introduce syntax a..<b which works exactly the same as a..b and only allow that version in match statements?

danielrab avatar Sep 23 '22 16:09 danielrab

I'll try to summarize in the hope this can move from experimental to stable.

  • in 2016 there was an initial concern of conflict with the syntax of slice patterns, and about the syntax of inclusive/exclusive patterns
  • in 2017 an exclusive pattern has been implemented but is blocked as experimental
  • since then, multiple comments stated that having the exclusive pattern would improve consistency in the language, all received more positive votes than negative ones
  • someone argued that 0..=9 wouldn't be less clear than 0..10, but a counter-example showed that some inclusive ranges were harder to read (with more positive votes overall)
  • a related issue on half-ranges https://github.com/rust-lang/rust/issues/67264 is still open but has passed the final comment period (seems complete?)
  • one comment above was not entirely clear to me, I think it suggests that patterns needn't look similar to expression, but there is nothing specific to illustrate this particular issue.

In conclusion, is there still anything blocking, or could this feature be accepted?

blueglyph avatar Nov 10 '22 11:11 blueglyph

🤷 36235598

dumindu avatar Nov 13 '22 16:11 dumindu

Adding to what @avl said:

In this case, having to write NOMINAL_LOW..=NOMINAL_HIGH-1 seems slightly worse. than NOMINAL_LOW..NOMINAL_HIGH .

Note that you can't even write NOMINAL_LOW..=NOMINAL_HIGH-1 in pattern position, because expressions don't work there. You can do something like

const NOMINAL_LOW: i32 = 0;
const NOMINAL_LOW_MAX: i32 = NOMINAL_LOW - 1;
const NOMINAL_HIGH: i32 = 100;
const NOMINAL_HIGH_MAX: i32 = NOMINAL_HIGH - 1;

and then have match arms like

NOMINAL_LOW..=NOMINAL_LOW_MAX => ...,
NOMINAL_HIGH..=NOMINAL_HIGH_MAX => ...,

but it is quite a bit noisier. I find myself running into this every once in a while when implementing file formats and things of that sort that involve magic number ranges.

There is a workaround which is somewhat worse for readability I think but perhaps the DRYest solution. You can make use of the fact that match arms are checked in order and use overlapping half-open ranges:

OVERVOLTAGE.. => "over"
NOMINAL_HIGH.. => "high",
NOMINAL_LOW.. => "low",
_ => "none",

The order now has to be backwards though, so I don't really love it.

whentze avatar Dec 15 '22 01:12 whentze

There is a workaround which is somewhat worse for readability I think but perhaps the DRYest solution. You can make use of the fact that match arms are checked in order and use overlapping half-open ranges:

OVERVOLTAGE.. => "over" NOMINAL_HIGH.. => "high", NOMINAL_LOW.. => "low", _ => "none", The order now has to be backwards though, so I don't really love it.

I like the tidiness of it, but aren't the open intervals a concern for robustness? Now the match cases are not sorted by priority but must be carefully sorted to get the "others" case at the end working correctly, and if a new value is inserted (for ex. between NOMINAL_LOW_MAX and NOMINAL_HIGH), it may break the match statement. Even searching for the *_MAX values' usage won't reveal it.

To come back on the difference - or confusion - between expression and pattern:

My understanding (for what it's worth) of a good language or API is that it doesn't show unnecessary exceptions and provides as best a coherency as possible. So even if one is an expression and the other a pattern, making an exception in the language by refusing the exclusive range in the latter can only confuse programmers, without any good reason to make up for it.

blueglyph avatar Dec 15 '22 09:12 blueglyph