+5

CHANGELOG.md

View file Reviewed

··· 243 243 ``` 244 244 245 245 ([Gavin Morrow](https://github.com/gavinmorrow)) 246 246 + 247 247 + - Fixed a bug where the JavaScript code generator could produce duplicate `let` 248 248 + declarations when a variable was reassigned after being shadowed inside a 249 249 + directly matching `case` branch. 250 250 + ([Eyup Can Akman](https://github.com/eyupcanakman))

+3 -18

compiler-core/src/javascript/decision.rs

View file Reviewed

··· 456 456 match &self.kind { 457 457 DecisionKind::Case { .. } => { 458 458 // Restore the user variables that were in scope before the 459 459 - // branch. The synthesised counters are left advanced. 459 459 + // branch. The synthesised counters are left advanced, and 460 460 + // the high-water marks keep any user variable that leaked 461 461 + // out of the branch from being redeclared by a later `let`. 460 462 self.variables 461 463 .expression_generator 462 464 .current_scope 463 465 .restore_user_variables(&old_user_variables); 464 464 - 465 465 - // For binded variables inside body we need to check if 466 466 - // there is same-named variable in old scope. In this case 467 467 - // we need to register next available variable, so no 468 468 - // redeclaration occurs. 469 469 - if let Decision::Run { body } = fallback { 470 470 - for (variable, _) in body.bindings.iter() { 471 471 - if old_user_variables.get(variable).is_some() { 472 472 - let new_variable_name = self.variables.next_local_var(variable); 473 473 - 474 474 - self.variables 475 475 - .expression_generator 476 476 - .current_scope 477 477 - .set_counter(&new_variable_name, 0); 478 478 - } 479 479 - } 480 480 - } 481 466 } 482 467 DecisionKind::LetAssert { .. } => {} 483 468 }

+20 -2

compiler-core/src/javascript/expression.rs

View file Reviewed

··· 144 144 #[derive(Debug, Clone, Default)] 145 145 pub(crate) struct Scope { 146 146 user_variables: im::HashMap<EcoString, usize>, 147 147 + /// The highest suffix handed out for each user variable still declared in 148 148 + /// the current JS scope, kept across a directly matching `case` branch so a 149 149 + /// variable that leaked out of it is not redeclared by a later `let`. 150 150 + high_water: im::HashMap<EcoString, usize>, 147 151 assignment: Option<usize>, 148 152 pipe: Option<usize>, 149 153 block: Option<usize>, ··· 191 195 _ => { 192 196 let _ = self.user_variables.insert(name.clone(), value); 193 197 } 198 198 + } 199 199 + } 200 200 + 201 201 + /// Advance the counter for a name to its next suffix, skipping any suffix 202 202 + /// already handed out for it in the current scope so a name that leaked out 203 203 + /// of a directly matching branch can't be redeclared. 204 204 + fn advance_counter(&mut self, name: &EcoString) { 205 205 + let in_scope = self.counter(name).map_or(0, |i| i + 1); 206 206 + let high_water = self.high_water.get(name).map_or(0, |i| i + 1); 207 207 + let next = in_scope.max(high_water); 208 208 + self.set_counter(name, next); 209 209 + // Only user variables leak out of a directly matching branch; the 210 210 + // synthesised counters survive the restore on their own. 211 211 + if self.user_variables.contains_key(name) { 212 212 + let _ = self.high_water.insert(name.clone(), next); 194 213 } 195 214 } 196 215 ··· 314 333 } 315 334 316 335 pub fn next_local_var(&mut self, name: &EcoString) -> EcoString { 317 317 - let next = self.current_scope.counter(name).map_or(0, |i| i + 1); 318 318 - self.current_scope.set_counter(name, next); 336 336 + self.current_scope.advance_counter(name); 319 337 self.local_var(name) 320 338 } 321 339

+20

compiler-core/src/javascript/tests/case.rs

View file Reviewed

··· 1061 1061 ) 1062 1062 } 1063 1063 1064 1064 + // https://github.com/gleam-lang/gleam/issues/5748 1065 1065 + #[test] 1066 1066 + fn no_duplicate_let_when_rebinding_variable_after_directly_matching_case() { 1067 1067 + assert_js!( 1068 1068 + r#" 1069 1069 + pub fn go() { 1070 1070 + let n = 1 1071 1071 + case True { 1072 1072 + True -> { 1073 1073 + let n = 99 1074 1074 + n 1075 1075 + } 1076 1076 + False -> 0 1077 1077 + } 1078 1078 + let n = n + 5 1079 1079 + n 1080 1080 + }"# 1081 1081 + ) 1082 1082 + } 1083 1083 + 1064 1084 #[test] 1065 1085 fn semicolon_exists_with_directly_matching_case() { 1066 1086 assert_js!(

+28

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__case__no_duplicate_let_when_rebinding_variable_after_directly_matching_case.snap

View file Reviewed

··· 1 1 + --- 2 2 + source: compiler-core/src/javascript/tests/case.rs 3 3 + expression: "\npub fn go() {\n let n = 1\n case True {\n True -> {\n let n = 99\n n\n }\n False -> 0\n }\n let n = n + 5\n n\n}" 4 4 + --- 5 5 + ----- SOURCE CODE 6 6 + 7 7 + pub fn go() { 8 8 + let n = 1 9 9 + case True { 10 10 + True -> { 11 11 + let n = 99 12 12 + n 13 13 + } 14 14 + False -> 0 15 15 + } 16 16 + let n = n + 5 17 17 + n 18 18 + } 19 19 + 20 20 + ----- COMPILED JAVASCRIPT 21 21 + export function go() { 22 22 + let n = 1; 23 23 + let $ = true; 24 24 + let n$1 = 99; 25 25 + n$1; 26 26 + let n$2 = n + 5; 27 27 + return n$2; 28 28 + }

+17

test/language/test/language/directly_matching_case_subject_test.gleam

View file Reviewed

··· 73 73 |> add_one 74 74 assert result == 2 75 75 } 76 76 + 77 77 + // https://github.com/gleam-lang/gleam/issues/5748 78 78 + pub fn no_duplicate_let_when_rebinding_variable_after_directly_matching_case_test() { 79 79 + let result = { 80 80 + let n = 1 81 81 + case True { 82 82 + True -> { 83 83 + let n = 99 84 84 + n 85 85 + } 86 86 + False -> 0 87 87 + } 88 88 + let n = n + 5 89 89 + n 90 90 + } 91 91 + assert result == 6 92 92 + }