From 72155b1de0f203e3dfe912bcbfc7045ec3b314e0 Mon Sep 17 00:00:00 2001 From: "Phoenix C. Enero" Date: Thu, 11 Aug 2016 15:43:46 +0800 Subject: [PATCH 1/8] Add 'else match' blocks to if expressions. --- text/0000-else-match.md | 228 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 228 insertions(+) create mode 100644 text/0000-else-match.md diff --git a/text/0000-else-match.md b/text/0000-else-match.md new file mode 100644 index 00000000000..3e047d39c21 --- /dev/null +++ b/text/0000-else-match.md @@ -0,0 +1,228 @@ +- Feature Name: Else Match +- Start Date: 2016-07-26 +- RFC PR: +- Rust Issue: + +# Summary +[summary]: #summary + +Extend the `if` expression to accept an `else match` block. + +# Motivation +[motivation]: #motivation + +This proposal is meant to reduce the verbosity of writing `if ... else { match ... } ` style +statements. + +It also makes code similar to: + +```rs +let flag = false; + +if foo() { + do_this(); +} else { + match bar() { + baz() => do_that(), + _ => flag = true + } +} + +if flag { + do_something(); +} +``` + +simpler and more concise: + +```rs +if foo() { + do_this(); +} else match bar() { + baz() => do_that() +} else { + do_something(); +} +``` + +## Use-cases + +Though rare, this pattern does exist in several Rust projects. + +### [Servo](https://github.com/servo/servo) + + + +Before: + +```rs +/// Clamp the given size by the given `min` and `max` constraints. +pub fn clamp(&self, other: Au) -> Au { + if other < self.min { + self.min + } else { + match self.max { + Some(max) if max < other => max, + _ => other + } + } +} +``` + +After: + +```rs +/// Clamp the given size by the given `min` and `max` constraints. +pub fn clamp(&self, other: Au) -> Au { + if other < self.min { + self.min + } else match self.max { + Some(max) if max < other => max, + _ => other + } +} +``` + +### [xsv](https://github.com/BurntSushi/xsv) + +: + +Before: + +```rs +if !self.typ.is_number() { + pieces.push(empty()); pieces.push(empty()); +} else { + match self.online { + Some(ref v) => { + pieces.push(v.mean().to_string()); + pieces.push(v.stddev().to_string()); + } + None => { pieces.push(empty()); pieces.push(empty()); } + } +} +``` + +After: + +```rs +if !self.typ.is_number() { + pieces.push(empty()); pieces.push(empty()); +} else match self.online { + Some(ref v) => { + pieces.push(v.mean().to_string()); + pieces.push(v.stddev().to_string()); + } + None => { pieces.push(empty()); pieces.push(empty()); } +} +``` + +### [trust-dns](https://github.com/bluejekyll/trust-dns) + +: + +Before: + +```rs +if class == self.class { + match rr.get_rr_type() { + RecordType::ANY | RecordType::AXFR | RecordType::IXFR => return Err(ResponseCode::FormErr), + _ => (), + } +} else { + match class { + DNSClass::ANY => { + if rr.get_ttl() != 0 { return Err(ResponseCode::FormErr) } + if let &RData::NULL(..) = rr.get_rdata() { () } + else { return Err(ResponseCode::FormErr) } + match rr.get_rr_type() { + RecordType::AXFR | RecordType::IXFR => return Err(ResponseCode::FormErr), + _ => (), + } + }, + DNSClass::NONE => { + if rr.get_ttl() != 0 { return Err(ResponseCode::FormErr) } + match rr.get_rr_type() { + RecordType::ANY | RecordType::AXFR | RecordType::IXFR => return Err(ResponseCode::FormErr), + _ => (), + } + }, + _ => return Err(ResponseCode::FormErr), + } +} +``` + +After: + +```rs +if class == self.class { + match rr.get_rr_type() { + RecordType::ANY | RecordType::AXFR | RecordType::IXFR => return Err(ResponseCode::FormErr), + _ => (), + } +} else match class { + DNSClass::ANY => { + if rr.get_ttl() != 0 { return Err(ResponseCode::FormErr) } + if let &RData::NULL(..) = rr.get_rdata() { () } + else { return Err(ResponseCode::FormErr) } + match rr.get_rr_type() { + RecordType::AXFR | RecordType::IXFR => return Err(ResponseCode::FormErr), + _ => (), + } + }, + DNSClass::NONE => { + if rr.get_ttl() != 0 { return Err(ResponseCode::FormErr) } + match rr.get_rr_type() { + RecordType::ANY | RecordType::AXFR | RecordType::IXFR => return Err(ResponseCode::FormErr), + _ => (), + } + }, +} else { + return Err(ResponseCode::FormErr) +} +``` + +# Detailed design +[design]: #detailed-design + +## Grammar + +See the following document for an (incomplete) guide to the grammar used in Rust: +[Rust Documentation → Grammar](https://doc.rust-lang.org/grammar.html). + +This proposal modifies the +[if expression grammar](https://doc.rust-lang.org/grammar.html#if-expressions). + +``` +else_tail : "else" [ if_expr | if_let_expr ++ | match_expr + | '{' block '}' ] ; +``` + +## Execution + +An `else match` block should be treated similar to an `else` block with a single `match` +expression, with one key addition: + +When an arbitrary expression `exp` in `else match ` fails to match any of the cases in the +`else match` block, the next block (if it exists) is run. + +### Dead code + +An `else match` block with a `_ => ...` match means the next clause (if exists) will never run. +There might be more complicated cases to optimize for, and is outside the scope of this document. + +# Drawbacks +[drawbacks]: #drawbacks + +- Slight maintainability problems whenever you need to add additional logic to an `else` block. + +# Alternatives +[alternatives]: #alternatives + +Don't do this. + +# Unresolved questions +[unresolved]: #unresolved-questions + +None. From 9c0c02f5189be8b178256e8cde4a22d4c83df8dc Mon Sep 17 00:00:00 2001 From: "Phoenix C. Enero" Date: Thu, 11 Aug 2016 15:51:03 +0800 Subject: [PATCH 2/8] Removed stray comma. --- text/0000-else-match.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-else-match.md b/text/0000-else-match.md index 3e047d39c21..79518dea9ad 100644 --- a/text/0000-else-match.md +++ b/text/0000-else-match.md @@ -210,7 +210,7 @@ When an arbitrary expression `exp` in `else match ` fails to match any of t ### Dead code An `else match` block with a `_ => ...` match means the next clause (if exists) will never run. -There might be more complicated cases to optimize for, and is outside the scope of this document. +There might be more complicated cases to optimize for and is outside the scope of this document. # Drawbacks [drawbacks]: #drawbacks From 36f23716d9da0e5886aae09396abc3acc9e91e46 Mon Sep 17 00:00:00 2001 From: "Phoenix C. Enero" Date: Thu, 11 Aug 2016 15:56:30 +0800 Subject: [PATCH 3/8] Add `if match` alternative. --- text/0000-else-match.md | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/text/0000-else-match.md b/text/0000-else-match.md index 79518dea9ad..d3191cc9898 100644 --- a/text/0000-else-match.md +++ b/text/0000-else-match.md @@ -178,7 +178,7 @@ if class == self.class { } }, } else { - return Err(ResponseCode::FormErr) + return Err(ResponseCode::FormErr); } ``` @@ -220,7 +220,18 @@ There might be more complicated cases to optimize for and is outside the scope o # Alternatives [alternatives]: #alternatives -Don't do this. +Not an alternative but an addition to the proposal: `if match` expressions. This would modify the +grammar as so: + +``` ++ if_match_expr : "if" match_expr else_tail ? ; + +else_tail : "else" [ if_expr | if_let_expr ++ | if_match_expr | match_expr + | '{' block '}' ] ; +``` + +Should work nearly the same as `else match`. # Unresolved questions [unresolved]: #unresolved-questions From 8e494b0eef43109ec5d099d3f22c279aa1e42ca7 Mon Sep 17 00:00:00 2001 From: "Phoenix C. Enero" Date: Thu, 11 Aug 2016 16:02:39 +0800 Subject: [PATCH 4/8] Fix grammar for the alternative `if match` addition --- text/0000-else-match.md | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/text/0000-else-match.md b/text/0000-else-match.md index d3191cc9898..6442fa75887 100644 --- a/text/0000-else-match.md +++ b/text/0000-else-match.md @@ -220,15 +220,21 @@ There might be more complicated cases to optimize for and is outside the scope o # Alternatives [alternatives]: #alternatives -Not an alternative but an addition to the proposal: `if match` expressions. This would modify the -grammar as so: +Not an alternative but an addition to the proposal: `if match` expressions. This would add an +additional grammar rule and modify an existing one: ``` -+ if_match_expr : "if" match_expr else_tail ? ; + expr : literal | path | tuple_expr | unit_expr | struct_expr + | block_expr | method_call_expr | field_expr | array_expr + | idx_expr | range_expr | unop_expr | binop_expr + | paren_expr | call_expr | lambda_expr | while_expr + | loop_expr | break_expr | continue_expr | for_expr + | if_expr | match_expr | if_let_expr | while_let_expr +~ | if_match_expr | return_expr ; -else_tail : "else" [ if_expr | if_let_expr -+ | if_match_expr | match_expr - | '{' block '}' ] ; +... + ++ if_match_expr : "if" match_expr else_tail ? ; ``` Should work nearly the same as `else match`. From 3ed3a9f9c7d1e5bc09ebf0af925e0cbc9f72f00c Mon Sep 17 00:00:00 2001 From: "Phoenix C. Enero" Date: Thu, 11 Aug 2016 16:06:31 +0800 Subject: [PATCH 5/8] Fix code snippet highlighting. --- text/0000-else-match.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/text/0000-else-match.md b/text/0000-else-match.md index 6442fa75887..91042bc0896 100644 --- a/text/0000-else-match.md +++ b/text/0000-else-match.md @@ -16,7 +16,7 @@ statements. It also makes code similar to: -```rs +```rust let flag = false; if foo() { @@ -35,7 +35,7 @@ if flag { simpler and more concise: -```rs +```rust if foo() { do_this(); } else match bar() { @@ -55,7 +55,7 @@ Though rare, this pattern does exist in several Rust projects. Before: -```rs +```rust /// Clamp the given size by the given `min` and `max` constraints. pub fn clamp(&self, other: Au) -> Au { if other < self.min { @@ -71,7 +71,7 @@ pub fn clamp(&self, other: Au) -> Au { After: -```rs +```rust /// Clamp the given size by the given `min` and `max` constraints. pub fn clamp(&self, other: Au) -> Au { if other < self.min { @@ -89,7 +89,7 @@ pub fn clamp(&self, other: Au) -> Au { Before: -```rs +```rust if !self.typ.is_number() { pieces.push(empty()); pieces.push(empty()); } else { @@ -105,7 +105,7 @@ if !self.typ.is_number() { After: -```rs +```rust if !self.typ.is_number() { pieces.push(empty()); pieces.push(empty()); } else match self.online { @@ -123,7 +123,7 @@ if !self.typ.is_number() { Before: -```rs +```rust if class == self.class { match rr.get_rr_type() { RecordType::ANY | RecordType::AXFR | RecordType::IXFR => return Err(ResponseCode::FormErr), @@ -154,7 +154,7 @@ if class == self.class { After: -```rs +```rust if class == self.class { match rr.get_rr_type() { RecordType::ANY | RecordType::AXFR | RecordType::IXFR => return Err(ResponseCode::FormErr), From 6f8972d093177040d2120563ee04ffebb31a7aca Mon Sep 17 00:00:00 2001 From: "Phoenix C. Enero" Date: Fri, 12 Aug 2016 10:30:26 +0800 Subject: [PATCH 6/8] Modified proposal for else match Removed an example, removed "match else"-style functionality, added some clarification, and slight tweaks to the formatting of the grammar. --- text/0000-else-match.md | 83 ++++++++++------------------------------- 1 file changed, 19 insertions(+), 64 deletions(-) diff --git a/text/0000-else-match.md b/text/0000-else-match.md index 91042bc0896..f017527c783 100644 --- a/text/0000-else-match.md +++ b/text/0000-else-match.md @@ -11,14 +11,9 @@ Extend the `if` expression to accept an `else match` block. # Motivation [motivation]: #motivation -This proposal is meant to reduce the verbosity of writing `if ... else { match ... } ` style -statements. - -It also makes code similar to: +This proposal is meant to reduce the verbosity of writing `if ... else { match ... } ` expressions. ```rust -let flag = false; - if foo() { do_this(); } else { @@ -27,10 +22,6 @@ if foo() { _ => flag = true } } - -if flag { - do_something(); -} ``` simpler and more concise: @@ -40,8 +31,6 @@ if foo() { do_this(); } else match bar() { baz() => do_that() -} else { - do_something(); } ``` @@ -49,40 +38,6 @@ if foo() { Though rare, this pattern does exist in several Rust projects. -### [Servo](https://github.com/servo/servo) - - - -Before: - -```rust -/// Clamp the given size by the given `min` and `max` constraints. -pub fn clamp(&self, other: Au) -> Au { - if other < self.min { - self.min - } else { - match self.max { - Some(max) if max < other => max, - _ => other - } - } -} -``` - -After: - -```rust -/// Clamp the given size by the given `min` and `max` constraints. -pub fn clamp(&self, other: Au) -> Au { - if other < self.min { - self.min - } else match self.max { - Some(max) if max < other => max, - _ => other - } -} -``` - ### [xsv](https://github.com/BurntSushi/xsv) : @@ -177,8 +132,7 @@ if class == self.class { _ => (), } }, -} else { - return Err(ResponseCode::FormErr); + _ => return Err(ResponseCode::FormErr), } ``` @@ -194,28 +148,27 @@ This proposal modifies the [if expression grammar](https://doc.rust-lang.org/grammar.html#if-expressions). ``` -else_tail : "else" [ if_expr | if_let_expr -+ | match_expr - | '{' block '}' ] ; + else_tail : "else" [ if_expr | if_let_expr ++ | match_expr + | '{' block '}' ] ; ``` ## Execution -An `else match` block should be treated similar to an `else` block with a single `match` -expression, with one key addition: - -When an arbitrary expression `exp` in `else match ` fails to match any of the cases in the -`else match` block, the next block (if it exists) is run. +An `else match` block should be treated exactly the same as an `else` block with a single `match` +expression. ### Dead code -An `else match` block with a `_ => ...` match means the next clause (if exists) will never run. -There might be more complicated cases to optimize for and is outside the scope of this document. +The next `else` block after an `else match` is never run. This is because `match` itself does not +take on an `else` block. Whether `match` should allow an `else` or not should be addressed in a +separate proposal. # Drawbacks [drawbacks]: #drawbacks - Slight maintainability problems whenever you need to add additional logic to an `else` block. +- Can be more of a stylistic issue than an ergonomics issue. # Alternatives [alternatives]: #alternatives @@ -223,6 +176,10 @@ There might be more complicated cases to optimize for and is outside the scope o Not an alternative but an addition to the proposal: `if match` expressions. This would add an additional grammar rule and modify an existing one: +``` ++ if_match_expr : "if" match_expr else_tail ? ; +``` + ``` expr : literal | path | tuple_expr | unit_expr | struct_expr | block_expr | method_call_expr | field_expr | array_expr @@ -230,11 +187,8 @@ additional grammar rule and modify an existing one: | paren_expr | call_expr | lambda_expr | while_expr | loop_expr | break_expr | continue_expr | for_expr | if_expr | match_expr | if_let_expr | while_let_expr -~ | if_match_expr | return_expr ; - -... - -+ if_match_expr : "if" match_expr else_tail ? ; +- | return_expr ; ++ | if_match_expr | return_expr ; ``` Should work nearly the same as `else match`. @@ -242,4 +196,5 @@ Should work nearly the same as `else match`. # Unresolved questions [unresolved]: #unresolved-questions -None. +Whether to allow `match` to take on an `else` block. This should be addressed +in a separate proposal. From 8feb0b7f0d02ace44877b412467bd9b79be9efac Mon Sep 17 00:00:00 2001 From: "Phoenix C. Enero" Date: Fri, 12 Aug 2016 10:32:36 +0800 Subject: [PATCH 7/8] Fixed sentences. --- text/0000-else-match.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/text/0000-else-match.md b/text/0000-else-match.md index f017527c783..0fac4495b82 100644 --- a/text/0000-else-match.md +++ b/text/0000-else-match.md @@ -13,6 +13,8 @@ Extend the `if` expression to accept an `else match` block. This proposal is meant to reduce the verbosity of writing `if ... else { match ... } ` expressions. +Code such as: + ```rust if foo() { do_this(); @@ -24,7 +26,7 @@ if foo() { } ``` -simpler and more concise: +can be simpler and more concise: ```rust if foo() { From 28df492947aa4250da33cb5f21e8169219d8cfca Mon Sep 17 00:00:00 2001 From: "Phoenix C. Enero" Date: Fri, 12 Aug 2016 10:33:20 +0800 Subject: [PATCH 8/8] Fixed code snippets --- text/0000-else-match.md | 1 + 1 file changed, 1 insertion(+) diff --git a/text/0000-else-match.md b/text/0000-else-match.md index 0fac4495b82..75faf49878d 100644 --- a/text/0000-else-match.md +++ b/text/0000-else-match.md @@ -33,6 +33,7 @@ if foo() { do_this(); } else match bar() { baz() => do_that() + _ => flag = true } ```