Skip to content

Commit

Permalink
fix(compiler): mutable lifted object may be modified inflight (#6258)
Browse files Browse the repository at this point in the history
fixes #3069
The type checker will now modify the type of preflight expressions that are being used inflight to a non-mutable version of the same type: `MutArray` -> `Array`, `MutJson` -> `Json`...

Note that since we change the type of the expression we don't get descriptive error telling us that we're trying to mutate a lifted mutable object, instead we just get the same error we'd expect if the lifted object was non-mutable to begin with. This isn't ideal but it makes the fix simpler. Let me know what you think...

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [x] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
yoav-steinberg authored Apr 17, 2024
1 parent 0e1222f commit d867af8
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 7 deletions.
5 changes: 2 additions & 3 deletions docs/docs/02-concepts/01-preflight-and-inflight.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
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).

```js playground
let var count = 3;
Expand All @@ -263,8 +263,7 @@ names.push("Jack"); // OK

inflight () => {
count = count + 1; // error: Variable cannot be reassigned from inflight
names.push("Jill"); // error: variable "names" cannot be mutated in inflight - error message not
// implemented yet, see https://github.com/winglang/wing/issues/3069
names.push("Jill"); // error: push doesn't exist in Array
};
```

Expand Down
30 changes: 30 additions & 0 deletions examples/tests/invalid/un_mut_lifted_objects.test.w
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
let ar = MutArray<num>[];
ar.push(1);

let j = MutJson {a: 1};
j.set("a", 2);

let st = MutSet<num>[1,2];
st.add(3);

let mp = MutMap<num>{"a" => 1};
mp.set("a", 2);

let opt_ar: MutArray<num>? = MutArray<num>[];
opt_ar?.push(1);

let recursive_ar = MutArray<MutArray<num>>[];
recursive_ar.push(MutArray<num>[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<num>[2]); // Error: push doesn't exist in Array
recursive_ar.at(0).push(3); // Error: push doesn't exist in Array
};
32 changes: 28 additions & 4 deletions libs/wingc/src/type_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2846,6 +2846,11 @@ 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();
Expand Down Expand Up @@ -4488,10 +4493,6 @@ 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_,
Expand Down Expand Up @@ -5691,6 +5692,29 @@ 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,
Expand Down
66 changes: 66 additions & 0 deletions tools/hangar/__snapshots__/invalid.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -4255,6 +4255,72 @@ error: Expected type to be \\"num\\", but got \\"str\\" instead



Tests 1 failed (1)
Snapshots 1 skipped
Test Files 1 failed (1)
Duration <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<num>[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)
Expand Down

0 comments on commit d867af8

Please sign in to comment.