fixup! Apply suggestions from code review

external-crates/move/documentation/book/src/control-flow.md

@@ -7,6 +7,6 @@ labels for loops and escapable named blocks. It also supports more more complex
 structural pattern matching.
 
 - [Conditional Expressions](./control-flow/conditionals.md)
+- [Pattern Matching](./control-flow/pattern-matching.md)
 - [Loops](./control-flow/loops.md)
 - [Labeled Control FLow](./control-flow/labeled-control-flow.md)
-- [Pattern Matching](./control-flow/pattern-matching.md)

external-crates/move/documentation/book/src/control-flow/pattern-matching.md

@@ -2,15 +2,13 @@
 
 A `match` expression is a powerful control structure that allows you to compare a value against a
 series of patterns and then execute code based on which pattern matches first. Patterns can be
-anything from simple literals to complex, nested struct and enum definitions . As opposed to `if` expressions, which change control flow based on a `bool`-typed test expression, a `match` expression operates over
-a value of any type and selects on of many arms.
+anything from simple literals to complex, nested struct and enum definitions . As opposed to `if`
+expressions, which change control flow based on a `bool`-typed test expression, a `match` expression
+operates over a value of any type and selects on of many arms.
 
 A `match` expression can match Move values, immutable references, or mutable references, binding
 sub-patterns accordingly.
 
-A pattern is matched by a value if the value is equal to the pattern, and where variables and
-wildcards (e.g., `x`, `y`, `_`, or `..`) are "equal" to anything.
-
 For example:
 
 ```move
@@ -43,29 +41,31 @@ match (expression) {
 }
 ```
 
-Match arms are checked in order from top to bottom, and the first pattern which matches
-(with a guard expression, if present, that evaluates to `true`) will be executed.
+Match arms are checked in order from top to bottom, and the first pattern which matches (with a
+guard expression, if present, that evaluates to `true`) will be executed.
 
 Note that the series of match arms within a `match` must be exhaustive, meaning that every possible
 value of the type being matched must be covered by one of the patterns in the `match`. If the series
 of match arms is not exhaustive, the compiler will raise an error.
 
 ## Patterns
 
-Patterns are used to match values. Patterns can be
-
-- literals (`true`, `2`, `@0x4`);
-- constants (`MyConstant`);
-- variables (`a`, `b`, `x`);
-- wildcards (`_`);
-- constructor patterns (`MyStruct { a, b }`, `MyEnum::Variant(x)`);
-- at-patterns `<variable> @ <pattern>`; and
-- or-patterns `<pattern> | <pattern>`.
+A pattern is matched by a value if the value is equal to the pattern, and where variables and
+wildcards (e.g., `x`, `y`, `_`, or `..`) are "equal" to anything.
 
-Additionally, depending on the context patterns may also include:
+Patterns are used to match values. Patterns can be
 
-- multi-arity wildcards (`..`); and
-- mutable-binding patterns (`mut x`).
+| Pattern              | Description                                                            |
+| -------------------- | ---------------------------------------------------------------------- |
+| Literal              | A literal value, e.g., `1`, `true`, `@0x1`                             |
+| Constant             | A constant value, e.g., `MyConstant`                                   |
+| Variable             | A variable, e.g., `x`, `y`, `z`                                        |
+| Wildcard             | A wildcard, e.g., `_`                                                  |
+| Constructor          | A constructor pattern, e.g., `MyStruct { x, y }`, `MyEnum::Variant(x)` |
+| At-pattern           | An at-pattern, e.g., `x @ MyEnum::Variant(..)`                         |
+| Or-pattern           | An or-pattern, e.g., `MyEnum::Variant(..) \| MyEnum::OtherVariant(..)` |
+| Multi-arity wildcard | A multi-arity wildcard, e.g., `MyEnum::Variant(..)`                    |
+| Mutable-binding      | A mutable-binding pattern, e.g., `mut x`                               |
 
 Some examples of patterns are:
 
@@ -114,7 +114,7 @@ x @ MyEnum::Variant(..)
 // or-pattern that matches either `MyEnum::Variant` or `MyEnum::OtherVariant`
 MyEnum::Variant(..) | MyEnum::OtherVariant(..)
 
-// Same as the above or-pattern, but with explicit wildcards
+// same as the above or-pattern, but with explicit wildcards
 MyEnum::Variant(_, _) | MyEnum::OtherVariant(_, _)
 
 // or-pattern that matches either `MyEnum::Variant` or `MyEnum::OtherVariant` and binds the u64 field to `x`
@@ -132,18 +132,18 @@ pattern = <literal>
         | <variable>
         | _
         | C { <variable> : inner-pattern ["," <variable> : inner-pattern]* } // where C is a struct or enum variant
-        | C ( inner-pattern ["," inner-pattern]* ... ) // where C is a struct or enum variant
-        | C                                       // where C is an enum variant
+        | C ( inner-pattern ["," inner-pattern]* ... )                       // where C is a struct or enum variant
+        | C                                                                  // where C is an enum variant
         | <variable> @ top-level-pattern
         | pattern | pattern
+        | mut <variable>
 inner-pattern = pattern
               | ..
-              | mut <variable>
 ```
 
-Patterns that contain variables bind them to the match subject or subject subcomponent being matched.
-These variables can then be
-used either in any match guard expressions, or on the right-hand side of the match arm. For example:
+Patterns that contain variables bind them to the match subject or subject subcomponent being
+matched. These variables can then be used either in any match guard expressions, or on the
+right-hand side of the match arm. For example:
 
 ```move
 public struct Wrapper(u64)
@@ -159,18 +159,16 @@ add_under_wrapper_unless_equal(Wrapper(2), 3); // returns Wrapper(5)
 add_under_wrapper_unless_equal(Wrapper(3), 3); // returns Wrapper(3)
 ```
 
-Patterns can be nested, and patterns can be combined used the or operator `|` which will succeed if either
-pattern matches. The `..` pattern is a special
-pattern that matches any number of fields in a struct or enum variant, but it can only occur within
-a constructor pattern, similarly the `mut` pattern can only be used within constructor patterns --
-this is used to specify that we want to use the variable mutably on the right-hand-side of the match
-arm.
+Patterns can be nested, and patterns can be combined used the or operator `|` which will succeed if
+either pattern matches. The `..` pattern is a special pattern that matches any number of fields in a
+struct or enum variant, but it can only occur once within a constructor pattern.
 
-Patterns are not expressions, but they are nevertheless typed. This means that
-the type of a pattern must match the type of the value it matches. For example, the pattern `1` has
-type `u64`, the pattern `MyEnum::Variant(1, true)` has type `MyEnum`, and the pattern
-`MyStruct { x, y }` has type `MyStruct`. If you try to match on an expression which differs from the
-type of the pattern in the match this will result in a type error. For example:
+Patterns are not expressions, but they are nevertheless typed. This means that the type of a pattern
+must match the type of the value it matches. For example, the pattern `1` has an integer type, the
+pattern `MyEnum::Variant(1, true)` has type `MyEnum`, and the pattern `MyStruct { x, y }` has type
+`MyStruct`, and `OtherStruct<bool> { x: true, y: 1}` has type `OtherStruct<bool>`. If you try to
+match on an expression which differs from the type of the pattern in the match this will result in a
+type error. For example:
 
 ```move
 match (1) {
@@ -194,37 +192,51 @@ match (MyStruct { x: 0, y: 0 }) {
 Additionally, there are some restrictions on when the `..` pattern, and `mut` pattern modifier can
 be used in a pattern.
 
-A `mut` modifier can only occur within a constructor pattern, and cannot be a top-level pattern. The
-value being matched on must be either a mutable reference or by value in order for a `mut` pattern
-to be used.
+A `mut` modifier can be placed on a variable pattern to specify that the _variable_ is to be mutated
+in the right-hand side expression of the match arm. Note that since the `mut` modifier only
+signifies that the variable is to be mutated, not the underlying data, this can be used on all types
+of match (by value, immutable reference, and mutable reference). Note that the `mut` modifier can
+only be applied to variables, and not other types of patterns.
 
 ```move
 public struct MyStruct(u64)
 
 fun top_level_mut(x: MyStruct) {
     match (x) {
         mut MyStruct(y) => 1,
-        // ERROR: cannot use mut pattern as a top-level pattern
+        // ERROR: cannot use mut on a non-variable pattern
     }
 }
 
-fun mut_on_non_mut(x: MyStruct): u64 {
+fun mut_on_immut(x: &MyStruct): u64 {
     match (x) {
+        MyStruct(mut y) => {
+            y = &(*y + 1);
+            *y
+        }
         // OK! Since `x` is matched by value
+    }
+}
+
+
+
+fun mut_on_non_mut(x: MyStruct): u64 {
+    match (x) {
         MyStruct(mut y) =>  {
             *y = *y + 1;
             *y
         },
+        // OK! Since `x` is matched by value
     }
 }
 
 fun mut_on_mut(x: &mut MyStruct): u64 {
     match (x) {
-        // OK! Since `x` is matched by mutable reference
         MyStruct(mut y) =>  {
             *y = *y + 1;
             *y
         },
+        // OK! Since `x` is matched by mutable reference
     }
 }
 
@@ -240,7 +252,12 @@ fun mut_on_immut(x: &MyStruct): u64 {
 }
 ```
 
-The `..` pattern an only be used within a constructor pattern and:
+The `..` pattern can only be used within a constructor pattern is a wildcard that matches any number
+of fields -- the  
+the compiler expands the `..` to inserting `_` in any missing fields in the constructor pattern (if
+any). So `MyStruct(_, _, _)` is the same as `MyStruct(..)`, `MyStruct(1, _, _)` is the same
+`MyStruct(1, ..)`. Because of this there are some restriction how, and where the `..` pattern can be
+used:
 
 - It can only be used **once** within the constructor pattern;
 - In positional arguments it can be used at the beginning, middle, or end of the patterns within the
@@ -325,13 +342,8 @@ fun test_or_pattern(x: u64): u64 {
         _ => 3,
     }
 }
-test_or_pattern(1); // returns 1
-test_or_pattern(2); // returns 1
 test_or_pattern(3); // returns 1
-test_or_pattern(4); // returns 2
 test_or_pattern(5); // returns 2
-test_or_pattern(6); // returns 2
-test_or_pattern(7); // returns 3
 test_or_pattern(70); // returns 3
 
 fun test_or_at_pattern(x: u64): u64 {
@@ -341,13 +353,8 @@ fun test_or_at_pattern(x: u64): u64 {
         z => z + 3,
     }
 }
-test_or_pattern(1); // returns 2
 test_or_pattern(2); // returns 3
-test_or_pattern(3); // returns 4
-test_or_pattern(4); // returns 6
 test_or_pattern(5); // returns 7
-test_or_pattern(6); // returns 8
-test_or_pattern(7); // returns 10
 test_or_pattern(70); // returns 73
 ```
 
@@ -390,7 +397,7 @@ it is `MyEnum::OtherVariant` with any value for the first field, and `3` for the
 `2`, if it is `MyEnum::Variant` with any fields, then return `3`, and if it is
 `MyEnum::OtherVariant` with any fields, then return `4`".
 
-You can also nest patterns, so if I wanted to match either 1, 2, or 10, instead of just matching 1
+You can also nest patterns, so if you wanted to match either 1, 2, or 10, instead of just matching 1
 in the `MyEnum::Variant` above, you could do so with an or-pattern:
 
 ```move
@@ -407,7 +414,12 @@ f(MyEnum::Variant(10, true)); // returns 1
 f(MyEnum::Variant(10, false)); // returns 3
 ```
 
-Additionally, match bindings are subject to the same ability restrictions as other aspects of Move. In particular, the compiler will signal an error if you try to match a value (i.e., not-reference) without `drop` using a wildcard, as the wildcard expects to drop the value. Similarly, if you bind a non-`drop` value using a binder, it must be used in the right-hand side of the match arm. In addition, if you fully-destruct that value, you have unpacked it, matching the semantics of  [non-`drop` struct unpacking](link). See [ref section] for more details about the `drop` capability. 
+Additionally, match bindings are subject to the same ability restrictions as other aspects of Move.
+In particular, the compiler will signal an error if you try to match a value (i.e., not-reference)
+without `drop` using a wildcard, as the wildcard expects to drop the value. Similarly, if you bind a
+non-`drop` value using a binder, it must be used in the right-hand side of the match arm. In
+addition, if you fully-destruct that value, you have unpacked it, matching the semantics of
+[non-`drop` struct unpacking](link). See [ref section] for more details about the `drop` capability.
 
 ```move
 public struct NonDrop(u64)
@@ -440,18 +452,18 @@ fun use_nondrop(x: NonDrop): NonDrop {
 
 The `match` expression in Move must be _exhaustive_: every possible value of the type being matched
 must be covered by one of the patterns in one of the match's arms. If the series of match arms is
-not exhaustive, the compiler will raise an error. Note that any arm with a guard expression
-does not contribute to match exhaustion, as it may fail to match at runtime.
+not exhaustive, the compiler will raise an error. Note that any arm with a guard expression does not
+contribute to match exhaustion, as it may fail to match at runtime.
 
 As an example, if we were to match on a `u8` then in order for the match to be exhaustive we would
-need to match on _every_ number from 0 to 255 inclusive, or a wildcard or variable pattern would need
-to be present. Similarly if we were to match on a `bool` then we would need to match on both `true`
-and `false`, or a wildcard or variable pattern would need to be present.
+need to match on _every_ number from 0 to 255 inclusive, or a wildcard or variable pattern would
+need to be present. Similarly if we were to match on a `bool` then we would need to match on both
+`true` and `false`, or a wildcard or variable pattern would need to be present.
 
 For structs, since there is only one type of constructor for the type, only one constructor needs to
 be matched, but the fields within the struct need to be matched exhaustively as well. Conversely,
-enums may define multiple variants, and each variant must be matched (including any sub-fields) in order for the match to be
-considered exhaustive.
+enums may define multiple variants, and each variant must be matched (including any sub-fields) in
+order for the match to be considered exhaustive.
 
 Since underscores and variables match anything, they count as matching all values of the type they
 are matching on in that position. Additionally, the multi-arity wildcard pattern `..` can be used to
@@ -524,7 +536,7 @@ checked.
 ```move
 fun match_with_guard(x: u64): u64 {
     match (x) {
-        1 if (x == 0) => 1,
+        1 if (false) => 1,
         1 => 2,
         _ => 3,
     }
@@ -534,10 +546,10 @@ match_with_guard(1); // returns 2
 match_with_guard(0); // returns 3
 ```
 
-Guard expressions can reference variables bound in the pattern during evaluation.
-However, note that _variables are only available as immutable reference in guards_ regardless
-of the pattern being matched -- even if there are mutability specifiers on the variable or if the
-pattern is being matched by value.
+Guard expressions can reference variables bound in the pattern during evaluation. However, note that
+_variables are only available as immutable reference in guards_ regardless of the pattern being
+matched -- even if there are mutability specifiers on the variable or if the pattern is being
+matched by value.
 
 ```move
 fun incr(x: &mut u64) {