Skip to content

Commit

Permalink
fix(compiler): no error when reassigning to non-reassignable variable (
Browse files Browse the repository at this point in the history
…#1196)

Additionally:
* import reassignability (let/var) from JSII class properties.
* added support for reassignable/non-reassignable function arguments.
* Verify reassignablity of class/resource fields.

see #1186

*By submitting this pull request, I confirm that my contribution is made
under the terms of the
[Monada Contribution
License](https://docs.winglang.io/terms-and-policies/contribution-license.html)*.

Co-authored-by: Chris Rybicki <[email protected]>
  • Loading branch information
yoav-steinberg and Chriscbr authored Jan 22, 2023
1 parent 149c730 commit 6c0026f
Show file tree
Hide file tree
Showing 15 changed files with 340 additions and 145 deletions.
13 changes: 13 additions & 0 deletions docs/04-reference/winglang-spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,19 @@ Examples in the class section below.
Assigning `var` to immutables of the same type is allowed. That is similar
to assigning non `readonly`s to `readonly`s in TypeScript.

By default function closure arguments are non-reassignable. By prefixing `var`
to an argument definition you can make a re-assignable function argument:

```ts
// wing
let f = (arg1: num, var arg2: num) => {
if (arg2 > 100) {
// We can reassign a value to arg2 since it's marked `var`
args2 = 100;
}
}
```

[`▲ top`][top]

---
Expand Down
21 changes: 21 additions & 0 deletions examples/tests/invalid/reassing_to_nonreassignable.w
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Assign to non-reassignable var
let x = 5;
x = x + 1;

// Assign to non-reassignable field
resource R {
f: num;
init() {
this.f = 1;
}

inc() {
this.f = this.f + 1;
}
}

// Assign to non-reassignable arg
let f = (arg: num):num => {
arg = 0;
return arg;
}
2 changes: 1 addition & 1 deletion examples/tests/valid/anon_function.w
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Define a function and assign it to a variable
let myfunc = (x: num) => {
let myfunc = (var x: num) => {
print("${x}");
x = x + 1;
if (x > 3.14) {
Expand Down
30 changes: 30 additions & 0 deletions examples/tests/valid/reassignment.w
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,33 @@ assert(x == 5);

x = x + 1;
assert(x == 6);

resource R {
var f: num;
f1: num;
init() {
// Initialize fields in `init` but in an inner scope to make sure
// we treat the special case of `this` access from init correctly at all scope levels
if true {
this.f = 1;
this.f1 = 0; // Access non-reassignable field from constructor is valid
}
}

inc() {
this.f = this.f + 1;
}
}

let r = new R();
r.inc();
assert(r.f == 2);

let f = (var arg: num):num => {
arg = 0;
return arg;
}

let y = 1;
assert(f(y) == 0);
assert(y == 1); // y is not modified, since Wing functions are pass-by-value
6 changes: 5 additions & 1 deletion libs/tree-sitter-wing/grammar.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,11 @@ module.exports = grammar({
access_modifier: ($) => choice("public", "private", "protected"),

parameter_definition: ($) =>
seq(field("name", $.identifier), $._type_annotation),
seq(
optional(field("reassignable", $.reassignable)),
field("name", $.identifier),
$._type_annotation
),

parameter_list: ($) => seq("(", commaSep($.parameter_definition), ")"),

Expand Down
3 changes: 2 additions & 1 deletion libs/tree-sitter-wing/test/corpus/statements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ return 1;
Inflight closure
====================

inflight (a: num, b: str?, c: bool) => {};
inflight (a: num, b: str?, var c: bool) => {};

---

Expand All @@ -95,6 +95,7 @@ inflight (a: num, b: str?, c: bool) => {};
)
)
(parameter_definition
reassignable: (reassignable)
name: (identifier)
type: (builtin_type)
)
Expand Down
17 changes: 8 additions & 9 deletions libs/wingc/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ pub struct FunctionSignature {
#[derive(Derivative)]
#[derivative(Debug)]
pub struct FunctionDefinition {
pub parameter_names: Vec<Symbol>, // TODO: move into FunctionSignature and make optional
// List of names of function parameters and whether they are reassignable (`var`) or not.
pub parameters: Vec<(Symbol, bool)>, // TODO: move into FunctionSignature and make optional

pub statements: Scope,
pub signature: FunctionSignature,
#[derivative(Debug = "ignore")]
Expand All @@ -143,7 +145,9 @@ pub struct FunctionDefinition {

#[derive(Debug)]
pub struct Constructor {
pub parameters: Vec<Symbol>,
// List of names of constructor parameters and whether they are reassignable (`var`) or not.
pub parameters: Vec<(Symbol, bool)>,

pub statements: Scope,
pub signature: FunctionSignature,
}
Expand Down Expand Up @@ -181,7 +185,7 @@ pub enum StmtKind {
identifier: Option<Symbol>,
},
VariableDef {
kind: VariableKind,
reassignable: bool,
var_name: Symbol,
initial_value: Expr,
type_: Option<Type>,
Expand Down Expand Up @@ -226,16 +230,11 @@ pub enum StmtKind {
},
}

#[derive(Debug, Clone, Copy, PartialEq)]
pub enum VariableKind {
Let,
Var,
}

#[derive(Debug)]
pub struct ClassMember {
pub name: Symbol,
pub member_type: Type,
pub reassignable: bool,
pub flight: Phase,
}

Expand Down
25 changes: 9 additions & 16 deletions libs/wingc/src/capture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,7 @@ pub fn scan_for_inflights_in_scope(scope: &Scope, diagnostics: &mut Diagnostics)
}
}
}
StmtKind::VariableDef {
kind: _,
var_name: _,
initial_value,
type_: _,
} => {
StmtKind::VariableDef { initial_value, .. } => {
scan_for_inflights_in_expression(initial_value, diagnostics);
}
StmtKind::Expression(exp) => {
Expand Down Expand Up @@ -195,7 +190,8 @@ fn scan_captures_in_call(
Ok((prop_type, phase)) => (
prop_type
.as_variable()
.expect("Expected resource property to be a variable"),
.expect("Expected resource property to be a variable")
._type,
phase,
),
Err(type_error) => {
Expand Down Expand Up @@ -254,7 +250,7 @@ fn scan_captures_in_expression(
// the type checker.

if x.is_ok() {
let (var, f) = x.unwrap();
let (var, si) = x.unwrap();

if var.as_variable().is_none() {
diagnostics.push(Diagnostic {
Expand All @@ -263,10 +259,10 @@ fn scan_captures_in_expression(
span: Some(symbol.span.clone()),
});
} else {
let t = var.as_variable().unwrap();
let t = var.as_variable().unwrap()._type;

// if the identifier represents a preflight value, then capture it
if f == Phase::Preflight {
if si.flight == Phase::Preflight {
if var.is_reassignable() {
diagnostics.push(Diagnostic {
level: DiagnosticLevel::Error,
Expand Down Expand Up @@ -405,12 +401,9 @@ fn scan_captures_in_inflight_scope(scope: &Scope, diagnostics: &mut Diagnostics)

for s in scope.statements.iter() {
match &s.kind {
StmtKind::VariableDef {
kind: _,
var_name: _,
initial_value,
type_: _,
} => res.extend(scan_captures_in_expression(initial_value, env, s.idx, diagnostics)),
StmtKind::VariableDef { initial_value, .. } => {
res.extend(scan_captures_in_expression(initial_value, env, s.idx, diagnostics))
}
StmtKind::ForLoop {
iterator: _,
iterable,
Expand Down
23 changes: 12 additions & 11 deletions libs/wingc/src/jsify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use sha2::{Digest, Sha256};
use crate::{
ast::{
ArgList, BinaryOperator, ClassMember, Expr, ExprKind, FunctionDefinition, InterpolatedStringPart, Literal, Phase,
Reference, Scope, Stmt, StmtKind, Symbol, Type, UnaryOperator, UtilityFunctions, VariableKind,
Reference, Scope, Stmt, StmtKind, Symbol, Type, UnaryOperator, UtilityFunctions,
},
capture::CaptureKind,
utilities::snake_case_to_camel_case,
Expand Down Expand Up @@ -438,7 +438,7 @@ impl JSifier {
ExprKind::FunctionClosure(func_def) => match func_def.signature.flight {
Phase::Inflight => self.jsify_inflight_function(func_def),
Phase::Independent => unimplemented!(),
Phase::Preflight => self.jsify_function(None, &func_def.parameter_names, &func_def.statements, phase),
Phase::Preflight => self.jsify_function(None, &func_def.parameters, &func_def.statements, phase),
},
}
}
Expand Down Expand Up @@ -466,15 +466,16 @@ impl JSifier {
)
}
StmtKind::VariableDef {
kind,
reassignable,
var_name,
initial_value,
type_: _,
} => {
let initial_value = self.jsify_expression(initial_value, phase);
return match kind {
VariableKind::Let => format!("const {} = {};", self.jsify_symbol(var_name), initial_value),
VariableKind::Var => format!("let {} = {};", self.jsify_symbol(var_name), initial_value),
return if *reassignable {
format!("let {} = {};", self.jsify_symbol(var_name), initial_value)
} else {
format!("const {} = {};", self.jsify_symbol(var_name), initial_value)
};
}
StmtKind::ForLoop {
Expand Down Expand Up @@ -567,7 +568,7 @@ impl JSifier {
.map(|(n, m)| format!(
"{} = {}",
n.name,
self.jsify_function(None, &m.parameter_names, &m.statements, phase)
self.jsify_function(None, &m.parameters, &m.statements, phase)
))
.collect::<Vec<String>>()
.join("\n")
Expand Down Expand Up @@ -623,8 +624,8 @@ impl JSifier {

fn jsify_inflight_function(&self, func_def: &FunctionDefinition) -> String {
let mut parameter_list = vec![];
for p in func_def.parameter_names.iter() {
parameter_list.push(p.name.clone());
for p in func_def.parameters.iter() {
parameter_list.push(p.0.name.clone());
}
let block = self.jsify_scope(&func_def.statements, Phase::Inflight);
let procid = base16ct::lower::encode_string(&Sha256::new().chain_update(&block).finalize());
Expand Down Expand Up @@ -696,10 +697,10 @@ impl JSifier {
)
}

fn jsify_function(&self, name: Option<&str>, parameters: &[Symbol], body: &Scope, phase: Phase) -> String {
fn jsify_function(&self, name: Option<&str>, parameters: &[(Symbol, bool)], body: &Scope, phase: Phase) -> String {
let mut parameter_list = vec![];
for p in parameters.iter() {
parameter_list.push(self.jsify_symbol(p));
parameter_list.push(self.jsify_symbol(&p.0));
}

let (name, arrow) = match name {
Expand Down
6 changes: 3 additions & 3 deletions libs/wingc/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#[macro_use]
extern crate lazy_static;

use ast::{Scope, Stmt, Symbol, UtilityFunctions, VariableKind};
use ast::{Scope, Stmt, Symbol, UtilityFunctions};
use diagnostic::{print_diagnostics, Diagnostic, DiagnosticLevel, Diagnostics, WingSpan};
use jsify::JSifier;
use type_check::symbol_env::StatementIdx;
Expand Down Expand Up @@ -130,7 +130,7 @@ pub fn parse(source_file: &str) -> (Scope, Diagnostics) {
}

pub fn type_check(scope: &mut Scope, types: &mut Types) -> Diagnostics {
let env = SymbolEnv::new(None, types.void(), false, Phase::Preflight, 0);
let env = SymbolEnv::new(None, types.void(), false, false, Phase::Preflight, 0);
scope.set_env(env);

add_builtin(
Expand Down Expand Up @@ -195,7 +195,7 @@ fn add_builtin(name: &str, typ: Type, scope: &mut Scope, types: &mut Types) {
.unwrap()
.define(
&sym,
SymbolKind::Variable(types.add_type(typ), VariableKind::Let),
SymbolKind::make_variable(types.add_type(typ), false),
StatementIdx::Top,
)
.expect("Failed to add builtin");
Expand Down
Loading

0 comments on commit 6c0026f

Please sign in to comment.