From 797829a3d3ef5f3b2244e975c856d21161e4bd16 Mon Sep 17 00:00:00 2001 From: Hasan <45375125+hasanaburayyan@users.noreply.github.com> Date: Wed, 17 Apr 2024 19:07:17 -0400 Subject: [PATCH] revert(compiler): mutable lifted object may be modified inflight (#6263) Reverts winglang/wing#6258 broke some winglibs due to strange behavior trying to call `copyMut` on immutable maps during inflight. See below image --- .../02-concepts/01-preflight-and-inflight.md | 5 +- .../invalid/un_mut_lifted_objects.test.w | 30 --------- libs/wingc/src/type_check.rs | 32 ++------- tools/hangar/__snapshots__/invalid.ts.snap | 66 ------------------- 4 files changed, 7 insertions(+), 126 deletions(-) delete mode 100644 examples/tests/invalid/un_mut_lifted_objects.test.w diff --git a/docs/docs/02-concepts/01-preflight-and-inflight.md b/docs/docs/02-concepts/01-preflight-and-inflight.md index 418fdef12ad..c3208888995 100644 --- a/docs/docs/02-concepts/01-preflight-and-inflight.md +++ b/docs/docs/02-concepts/01-preflight-and-inflight.md @@ -252,7 +252,7 @@ new cloud.Function(checkEndpoint); ``` However, mutation to preflight data is not allowed. -This mean means that variables from preflight cannot be reassigned to, and mutable collections like `MutArray` and `MutMap` cannot be modified (they're turned into their immutable counterparts, `Array` and `Map`, respectively when accessed inflight). +This mean means that variables from preflight cannot be reassigned to, and mutable collections like `MutArray` and `MutMap` cannot be modified. ```js playground let var count = 3; @@ -263,7 +263,8 @@ names.push("Jack"); // OK inflight () => { count = count + 1; // error: Variable cannot be reassigned from inflight - names.push("Jill"); // error: push doesn't exist in Array + names.push("Jill"); // error: variable "names" cannot be mutated in inflight - error message not + // implemented yet, see https://github.com/winglang/wing/issues/3069 }; ``` diff --git a/examples/tests/invalid/un_mut_lifted_objects.test.w b/examples/tests/invalid/un_mut_lifted_objects.test.w deleted file mode 100644 index cf42bafa1c0..00000000000 --- a/examples/tests/invalid/un_mut_lifted_objects.test.w +++ /dev/null @@ -1,30 +0,0 @@ -let ar = MutArray[]; -ar.push(1); - -let j = MutJson {a: 1}; -j.set("a", 2); - -let st = MutSet[1,2]; -st.add(3); - -let mp = MutMap{"a" => 1}; -mp.set("a", 2); - -let opt_ar: MutArray? = MutArray[]; -opt_ar?.push(1); - -let recursive_ar = MutArray>[]; -recursive_ar.push(MutArray[1]); -recursive_ar.at(0).push(2); - -inflight() => { - // Same code as above should be an error in inflight - ar.push(2); // Error: push doesn't exist in Array - ar[0] = 1; // Error: Cannot update elements of an immutable Array - j.set("a", 3); // Error: set doesn't exist in Json - st.add(4); // Error: add doesn't exist in Set - mp.set("a", 3); // Error: set doesn't exist in Map - opt_ar?.push(2); // Error: push doesn't exist in Array - recursive_ar.push(MutArray[2]); // Error: push doesn't exist in Array - recursive_ar.at(0).push(3); // Error: push doesn't exist in Array -}; diff --git a/libs/wingc/src/type_check.rs b/libs/wingc/src/type_check.rs index 524cac63e6d..4518c5d76cf 100644 --- a/libs/wingc/src/type_check.rs +++ b/libs/wingc/src/type_check.rs @@ -2846,11 +2846,6 @@ impl<'a> TypeChecker<'a> { } }(exp, env); - // If we're inflight but the expression is a lifted (preflight) expression then make it immutable - if env.phase == Phase::Inflight && phase == Phase::Preflight { - t = self.make_immutable(t); - } - self.types.assign_type_to_expr(exp, t, phase); self.curr_expr_info.pop(); @@ -4493,6 +4488,10 @@ impl<'a> TypeChecker<'a> { fn type_check_assignment(&mut self, kind: &AssignmentKind, value: &Expr, variable: &Reference, env: &mut SymbolEnv) { let (exp_type, _) = self.type_check_exp(value, env); + + // TODO: we need to verify that if this variable is defined in a parent environment (i.e. + // being captured) it cannot be reassigned: https://github.com/winglang/wing/issues/3069 + let (var, var_phase) = self.resolve_reference(&variable, env, false); let var_type = match &var { ResolveReferenceResult::Variable(var) => var.type_, @@ -5692,29 +5691,6 @@ impl<'a> TypeChecker<'a> { .map(|_| base_udt) } - fn make_immutable(&mut self, type_: TypeRef) -> TypeRef { - match *type_ { - Type::MutArray(inner) => { - let inner = self.make_immutable(inner); - self.types.add_type(Type::Array(inner)) - } - Type::MutJson => self.types.json(), - Type::MutMap(inner) => { - let inner = self.make_immutable(inner); - self.types.add_type(Type::Map(inner)) - } - Type::MutSet(inner) => { - let inner = self.make_immutable(inner); - self.types.add_type(Type::Set(inner)) - } - Type::Optional(inner) => { - let inner = self.make_immutable(inner); - self.types.add_type(Type::Optional(inner)) - } - _ => type_, - } - } - fn resolve_reference( &mut self, reference: &Reference, diff --git a/tools/hangar/__snapshots__/invalid.ts.snap b/tools/hangar/__snapshots__/invalid.ts.snap index ebfa0d9e5b3..b605cb048ae 100644 --- a/tools/hangar/__snapshots__/invalid.ts.snap +++ b/tools/hangar/__snapshots__/invalid.ts.snap @@ -4255,72 +4255,6 @@ error: Expected type to be \\"num\\", but got \\"str\\" instead -Tests 1 failed (1) -Snapshots 1 skipped -Test Files 1 failed (1) -Duration " -`; - -exports[`un_mut_lifted_objects.test.w 1`] = ` -"error: Member \\"push\\" doesn't exist in \\"Array\\" - --> ../../../examples/tests/invalid/un_mut_lifted_objects.test.w:22:6 - | -22 | ar.push(2); // Error: push doesn't exist in Array - | ^^^^ Member \\"push\\" doesn't exist in \\"Array\\" - - -error: Cannot update elements of an immutable Array - --> ../../../examples/tests/invalid/un_mut_lifted_objects.test.w:23:3 - | -23 | ar[0] = 1; // Error: Cannot update elements of an immutable Array - | ^^^^^ Cannot update elements of an immutable Array - | - = hint: Consider using MutArray instead - - -error: Member \\"set\\" doesn't exist in \\"Json\\" - --> ../../../examples/tests/invalid/un_mut_lifted_objects.test.w:24:5 - | -24 | j.set(\\"a\\", 3); // Error: set doesn't exist in Json - | ^^^ Member \\"set\\" doesn't exist in \\"Json\\" - - -error: Member \\"add\\" doesn't exist in \\"Set\\" - --> ../../../examples/tests/invalid/un_mut_lifted_objects.test.w:25:6 - | -25 | st.add(4); // Error: add doesn't exist in Set - | ^^^ Member \\"add\\" doesn't exist in \\"Set\\" - - -error: Member \\"set\\" doesn't exist in \\"Map\\" - --> ../../../examples/tests/invalid/un_mut_lifted_objects.test.w:26:6 - | -26 | mp.set(\\"a\\", 3); // Error: set doesn't exist in Map - | ^^^ Member \\"set\\" doesn't exist in \\"Map\\" - - -error: Member \\"push\\" doesn't exist in \\"Array\\" - --> ../../../examples/tests/invalid/un_mut_lifted_objects.test.w:27:11 - | -27 | opt_ar?.push(2); // Error: push doesn't exist in Array - | ^^^^ Member \\"push\\" doesn't exist in \\"Array\\" - - -error: Member \\"push\\" doesn't exist in \\"Array\\" - --> ../../../examples/tests/invalid/un_mut_lifted_objects.test.w:28:16 - | -28 | recursive_ar.push(MutArray[2]); // Error: push doesn't exist in Array - | ^^^^ Member \\"push\\" doesn't exist in \\"Array\\" - - -error: Member \\"push\\" doesn't exist in \\"Array\\" - --> ../../../examples/tests/invalid/un_mut_lifted_objects.test.w:29:22 - | -29 | recursive_ar.at(0).push(3); // Error: push doesn't exist in Array - | ^^^^ Member \\"push\\" doesn't exist in \\"Array\\" - - - Tests 1 failed (1) Snapshots 1 skipped Test Files 1 failed (1)