Permit binding variables multiple itmes in when branches

crates/compiler/can/src/annotation.rs

@@ -621,11 +621,11 @@ fn can_annotation_help(
             let symbol = match scope.introduce(name.value.into(), region) {
                 Ok(symbol) => symbol,
 
-                Err((original_region, shadow, _new_symbol)) => {
+                Err((shadowed_symbol, shadow, _new_symbol)) => {
-                    let problem = Problem::Shadowed(original_region, shadow.clone());
+                    let problem = Problem::Shadowed(shadowed_symbol.region, shadow.clone());
 
                     env.problem(roc_problem::can::Problem::Shadowing {
-                        original_region,
+                        original_region: shadowed_symbol.region,
                         shadow,
                         kind: ShadowKind::Variable,
                     });

crates/compiler/can/src/def.rs

@@ -2312,9 +2312,9 @@ fn to_pending_type_def<'a>(
 
                 let member_sym = match scope.introduce(member_name.into(), name_region) {
                     Ok(sym) => sym,
-                    Err((original_region, shadow, _new_symbol)) => {
+                    Err((shadowed_symbol, shadow, _new_symbol)) => {
                         env.problem(roc_problem::can::Problem::Shadowing {
-                            original_region,
+                            original_region: shadowed_symbol.region,
                             shadow,
                             kind: ShadowKind::Variable,
                         });

crates/compiler/can/src/expr.rs

@@ -7,7 +7,7 @@ use crate::num::{
     finish_parsing_base, finish_parsing_float, finish_parsing_num, float_expr_from_result,
     int_expr_from_result, num_expr_from_result, FloatBound, IntBound, NumBound,
 };
-use crate::pattern::{canonicalize_pattern, BindingsFromPattern, Pattern};
+use crate::pattern::{canonicalize_pattern, BindingsFromPattern, Pattern, PermitShadows};
 use crate::procedure::References;
 use crate::scope::Scope;
 use crate::traverse::{walk_expr, Visitor};
@@ -1187,6 +1187,7 @@ fn canonicalize_closure_body<'a>(
             FunctionArg,
             &loc_pattern.value,
             loc_pattern.region,
+            PermitShadows(false),
         );
 
         can_args.push((
@@ -1290,6 +1291,7 @@ fn canonicalize_when_branch<'a>(
             WhenBranch,
             &loc_pattern.value,
             loc_pattern.region,
+            PermitShadows(true),
         );
 
         patterns.push(can_pattern);

crates/compiler/can/src/pattern.rs

@@ -227,10 +227,25 @@ pub fn canonicalize_def_header_pattern<'a>(
                 }
             }
         }
-        _ => canonicalize_pattern(env, var_store, scope, output, pattern_type, pattern, region),
+        _ => canonicalize_pattern(
+            env,
+            var_store,
+            scope,
+            output,
+            pattern_type,
+            pattern,
+            region,
+            PermitShadows(false),
+        ),
     }
 }
 
+/// Allow binding of symbols that appear shadowed.
+///
+/// For example, in the branch `A x | B x -> ...`, both pattern bind `x`; that's not a shadow!
+#[derive(PartialEq, Eq, Clone, Copy)]
+pub struct PermitShadows(pub bool);
+
 pub fn canonicalize_pattern<'a>(
     env: &mut Env<'a>,
     var_store: &mut VarStore,
@@ -239,6 +254,7 @@ pub fn canonicalize_pattern<'a>(
     pattern_type: PatternType,
     pattern: &ast::Pattern<'a>,
     region: Region,
+    permit_shadows: PermitShadows,
 ) -> Loc<Pattern> {
     use roc_parse::ast::Pattern::*;
     use PatternType::*;
@@ -250,15 +266,21 @@ pub fn canonicalize_pattern<'a>(
 
                 Pattern::Identifier(symbol)
             }
-            Err((original_region, shadow, new_symbol)) => {
+            Err((shadowed_symbol, shadow, new_symbol)) => {
-                env.problem(Problem::RuntimeError(RuntimeError::Shadowing {
+                if permit_shadows.0 {
-                    original_region,
+                    output.references.insert_bound(shadowed_symbol.value);
-                    shadow: shadow.clone(),
-                    kind: ShadowKind::Variable,
-                }));
-                output.references.insert_bound(new_symbol);
 
-                Pattern::Shadowed(original_region, shadow, new_symbol)
+                    Pattern::Identifier(shadowed_symbol.value)
+                } else {
+                    env.problem(Problem::RuntimeError(RuntimeError::Shadowing {
+                        original_region: shadowed_symbol.region,
+                        shadow: shadow.clone(),
+                        kind: ShadowKind::Variable,
+                    }));
+                    output.references.insert_bound(new_symbol);
+
+                    Pattern::Shadowed(shadowed_symbol.region, shadow, new_symbol)
+                }
             }
         },
         Tag(name) => {
@@ -289,6 +311,7 @@ pub fn canonicalize_pattern<'a>(
                     pattern_type,
                     &loc_pattern.value,
                     loc_pattern.region,
+                    permit_shadows,
                 );
 
                 can_patterns.push((var_store.fresh(), can_pattern));
@@ -457,6 +480,7 @@ pub fn canonicalize_pattern<'a>(
                 pattern_type,
                 sub_pattern,
                 region,
+                permit_shadows,
             )
         }
         RecordDestructure(patterns) => {
@@ -482,9 +506,9 @@ pub fn canonicalize_pattern<'a>(
                                     },
                                 });
                             }
-                            Err((original_region, shadow, new_symbol)) => {
+                            Err((shadowed_symbol, shadow, new_symbol)) => {
                                 env.problem(Problem::RuntimeError(RuntimeError::Shadowing {
-                                    original_region,
+                                    original_region: shadowed_symbol.region,
                                     shadow: shadow.clone(),
                                     kind: ShadowKind::Variable,
                                 }));
@@ -493,8 +517,11 @@ pub fn canonicalize_pattern<'a>(
                                 // are, we're definitely shadowed and will
                                 // get a runtime exception as soon as we
                                 // encounter the first bad pattern.
-                                opt_erroneous =
+                                opt_erroneous = Some(Pattern::Shadowed(
-                                    Some(Pattern::Shadowed(original_region, shadow, new_symbol));
+                                    shadowed_symbol.region,
+                                    shadow,
+                                    new_symbol,
+                                ));
                             }
                         };
                     }
@@ -511,6 +538,7 @@ pub fn canonicalize_pattern<'a>(
                             pattern_type,
                             &loc_guard.value,
                             loc_guard.region,
+                            permit_shadows,
                         );
 
                         destructs.push(Loc {
@@ -550,9 +578,9 @@ pub fn canonicalize_pattern<'a>(
                                     },
                                 });
                             }
-                            Err((original_region, shadow, new_symbol)) => {
+                            Err((shadowed_symbol, shadow, new_symbol)) => {
                                 env.problem(Problem::RuntimeError(RuntimeError::Shadowing {
-                                    original_region,
+                                    original_region: shadowed_symbol.region,
                                     shadow: shadow.clone(),
                                     kind: ShadowKind::Variable,
                                 }));
@@ -561,8 +589,11 @@ pub fn canonicalize_pattern<'a>(
                                 // are, we're definitely shadowed and will
                                 // get a runtime exception as soon as we
                                 // encounter the first bad pattern.
-                                opt_erroneous =
+                                opt_erroneous = Some(Pattern::Shadowed(
-                                    Some(Pattern::Shadowed(original_region, shadow, new_symbol));
+                                    shadowed_symbol.region,
+                                    shadow,
+                                    new_symbol,
+                                ));
                             }
                         };
                     }

crates/compiler/can/src/scope.rs

@@ -253,7 +253,7 @@ impl Scope {
         &mut self,
         ident: Ident,
         region: Region,
-    ) -> Result<Symbol, (Region, Loc<Ident>, Symbol)> {
+    ) -> Result<Symbol, (Loc<Symbol>, Loc<Ident>, Symbol)> {
         self.introduce_str(ident.as_str(), region)
     }
 
@@ -261,17 +261,17 @@ impl Scope {
         &mut self,
         ident: &str,
         region: Region,
-    ) -> Result<Symbol, (Region, Loc<Ident>, Symbol)> {
+    ) -> Result<Symbol, (Loc<Symbol>, Loc<Ident>, Symbol)> {
         match self.introduce_help(ident, region) {
             Ok(symbol) => Ok(symbol),
-            Err((_, original_region)) => {
+            Err((shadowed_symbol, original_region)) => {
                 let shadow = Loc {
                     value: Ident::from(ident),
                     region,
                 };
                 let symbol = self.locals.scopeless_symbol(ident, region);
 
-                Err((original_region, shadow, symbol))
+                Err((Loc::at(original_region, shadowed_symbol), shadow, symbol))
             }
         }
     }
@@ -638,13 +638,13 @@ mod test {
         assert!(scope.lookup(&ident, Region::zero()).is_err());
 
         let first = scope.introduce(ident.clone(), region1).unwrap();
-        let (original_region, _ident, shadow_symbol) =
+        let (original, _ident, shadow_symbol) =
             scope.introduce(ident.clone(), region2).unwrap_err();
 
         scope.register_debug_idents();
 
         assert_ne!(first, shadow_symbol);
-        assert_eq!(original_region, region1);
+        assert_eq!(original.region, region1);
 
         let lookup = scope.lookup(&ident, Region::zero()).unwrap();
 
@@ -805,13 +805,13 @@ mod test {
 
         scope.import(ident.clone(), symbol, region1).unwrap();
 
-        let (original_region, _ident, shadow_symbol) =
+        let (original, _ident, shadow_symbol) =
             scope.introduce(ident.clone(), region2).unwrap_err();
 
         scope.register_debug_idents();
 
         assert_ne!(symbol, shadow_symbol);
-        assert_eq!(original_region, region1);
+        assert_eq!(original.region, region1);
 
         let lookup = scope.lookup(&ident, Region::zero()).unwrap();
 

crates/compiler/solve/tests/solve_expr.rs

@@ -7357,4 +7357,18 @@ mod solve_expr {
             "List (A U8)",
         );
     }
+
+    #[test]
+    fn shared_pattern_variable_in_when_branches() {
+        infer_eq_without_problem(
+            indoc!(
+                r#"
+                when A "" is
+                    A x | B x -> x
+                    C y | D y -> y
+                "#
+            ),
+            "",
+        );
+    }
 }