Skip to content

Commit

Permalink
fix: return statement message from void functions
Browse files Browse the repository at this point in the history
  • Loading branch information
Elad Cohen committed Jun 27, 2023
1 parent 5224e27 commit a62650a
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 22 deletions.
22 changes: 22 additions & 0 deletions examples/tests/invalid/return_types.w
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
return 9;
// ^^^^^^^^^ Return statement outside of function cannot return a value
if true {
return 9;
// ^^^^^^^^^ Return statement outside of function cannot return a value
}

inflight (): void => {
return 9;
//^^^^^^^^^ Unexpected return value from void function
if true {
return 9;
// ^^^^^^^^^ Unexpected return value from void function
}
};

class C {
func() {
return 9;
// ^^^^^^^^^ Unexpected return value from void function
}
}
2 changes: 1 addition & 1 deletion libs/wingc/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ pub fn type_check(
jsii_imports: &mut Vec<JsiiImportSpec>,
) {
assert!(scope.env.borrow().is_none(), "Scope should not have an env yet");
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);

// note: Globals are emitted here and wrapped in "{ ... }" blocks. Wrapping makes these emissions, actual
Expand Down
42 changes: 30 additions & 12 deletions libs/wingc/src/type_check.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,7 @@ impl Types {
// TODO: this is hack to create the top-level mapping from lib names to symbols
// We construct a void ref by hand since we can't call self.void() while constructing the Types struct
let void_ref = UnsafeRef::<Type>(&*types[void_idx] as *const Type);
let libraries = SymbolEnv::new(None, void_ref, false, Phase::Preflight, 0);
let libraries = SymbolEnv::new(None, void_ref, false, false, Phase::Preflight, 0);

Self {
types,
Expand Down Expand Up @@ -1794,6 +1794,7 @@ impl<'a> TypeChecker<'a> {
Some(env.get_ref()),
sig.return_type,
false,
true,
func_def.signature.phase,
self.statement_idx,
);
Expand Down Expand Up @@ -2133,7 +2134,7 @@ impl<'a> TypeChecker<'a> {
_t => self.types.error(),
};

let mut scope_env = SymbolEnv::new(Some(env.get_ref()), env.return_type, false, env.phase, stmt.idx);
let mut scope_env = SymbolEnv::new(Some(env.get_ref()), env.return_type, false, false, env.phase, stmt.idx);
match scope_env.define(
&iterator,
SymbolKind::make_free_variable(iterator.clone(), iterator_type, false, env.phase),
Expand All @@ -2156,6 +2157,7 @@ impl<'a> TypeChecker<'a> {
Some(env.get_ref()),
env.return_type,
false,
false,
env.phase,
stmt.idx,
));
Expand Down Expand Up @@ -2184,7 +2186,7 @@ impl<'a> TypeChecker<'a> {
// and complete the type checking process for additional errors.
let var_type = cond_type.maybe_unwrap_option();

let mut stmt_env = SymbolEnv::new(Some(env.get_ref()), env.return_type, false, env.phase, stmt.idx);
let mut stmt_env = SymbolEnv::new(Some(env.get_ref()), env.return_type, false, false, env.phase, stmt.idx);

// Add the variable to if block scope
match stmt_env.define(
Expand All @@ -2206,6 +2208,7 @@ impl<'a> TypeChecker<'a> {
Some(env.get_ref()),
env.return_type,
false,
false,
env.phase,
stmt.idx,
));
Expand All @@ -2225,6 +2228,7 @@ impl<'a> TypeChecker<'a> {
Some(env.get_ref()),
env.return_type,
false,
false,
env.phase,
stmt.idx,
));
Expand All @@ -2238,6 +2242,7 @@ impl<'a> TypeChecker<'a> {
Some(env.get_ref()),
env.return_type,
false,
false,
env.phase,
stmt.idx,
));
Expand All @@ -2249,6 +2254,7 @@ impl<'a> TypeChecker<'a> {
Some(env.get_ref()),
env.return_type,
false,
false,
env.phase,
stmt.idx,
));
Expand Down Expand Up @@ -2328,6 +2334,7 @@ impl<'a> TypeChecker<'a> {
Some(env.get_ref()),
env.return_type,
false,
false,
env.phase,
stmt.idx,
));
Expand All @@ -2338,6 +2345,8 @@ impl<'a> TypeChecker<'a> {
let return_type = self.type_check_exp(return_expression, env);
if !env.return_type.is_void() {
self.validate_type(return_type, env.return_type, return_expression);
} else if env.is_in_function() {
self.spanned_error(stmt, "Unexpected return value from void function");
} else {
self.spanned_error(stmt, "Return statement outside of function cannot return a value");
}
Expand Down Expand Up @@ -2369,7 +2378,7 @@ impl<'a> TypeChecker<'a> {
let (parent_class, parent_class_env) = self.extract_parent_class(parent.as_ref(), *phase, name, env, stmt);

// Create environment representing this class, for now it'll be empty just so we can support referencing ourselves from the class definition.
let dummy_env = SymbolEnv::new(None, self.types.void(), false, env.phase, stmt.idx);
let dummy_env = SymbolEnv::new(None, self.types.void(), false, false, env.phase, stmt.idx);

let impl_interfaces = implements
.iter()
Expand Down Expand Up @@ -2407,7 +2416,7 @@ impl<'a> TypeChecker<'a> {
};

// Create a the real class environment to be filled with the class AST types
let mut class_env = SymbolEnv::new(parent_class_env, self.types.void(), false, env.phase, stmt.idx);
let mut class_env = SymbolEnv::new(parent_class_env, self.types.void(), false, false, env.phase, stmt.idx);

// Add fields to the class env
for field in fields.iter() {
Expand Down Expand Up @@ -2558,7 +2567,7 @@ impl<'a> TypeChecker<'a> {
}
StmtKind::Interface(AstInterface { name, methods, extends }) => {
// Create environment representing this interface, for now it'll be empty just so we can support referencing ourselves from the interface definition.
let dummy_env = SymbolEnv::new(None, self.types.void(), false, env.phase, stmt.idx);
let dummy_env = SymbolEnv::new(None, self.types.void(), false, false, env.phase, stmt.idx);

let extend_interfaces = extends
.iter()
Expand Down Expand Up @@ -2594,7 +2603,7 @@ impl<'a> TypeChecker<'a> {
};

// Create the real interface environment to be filled with the interface AST types
let mut interface_env = SymbolEnv::new(None, self.types.void(), false, env.phase, stmt.idx);
let mut interface_env = SymbolEnv::new(None, self.types.void(), false, false, env.phase, stmt.idx);

// Add methods to the interface env
for (method_name, sig) in methods.iter() {
Expand Down Expand Up @@ -2632,7 +2641,7 @@ impl<'a> TypeChecker<'a> {
// fail type checking.

// Create an environment for the struct
let mut struct_env = SymbolEnv::new(None, self.types.void(), false, env.phase, stmt.idx);
let mut struct_env = SymbolEnv::new(None, self.types.void(), false, false, env.phase, stmt.idx);

// Add fields to the struct env
for field in fields.iter() {
Expand Down Expand Up @@ -2706,13 +2715,13 @@ impl<'a> TypeChecker<'a> {
finally_statements,
} => {
// Create a new environment for the try block
let try_env = SymbolEnv::new(Some(env.get_ref()), env.return_type, false, env.phase, stmt.idx);
let try_env = SymbolEnv::new(Some(env.get_ref()), env.return_type, false, false, env.phase, stmt.idx);
try_statements.set_env(try_env);
self.inner_scopes.push(try_statements);

// Create a new environment for the catch block
if let Some(catch_block) = catch_block {
let mut catch_env = SymbolEnv::new(Some(env.get_ref()), env.return_type, false, env.phase, stmt.idx);
let mut catch_env = SymbolEnv::new(Some(env.get_ref()), env.return_type, false, false, env.phase, stmt.idx);

// Add the exception variable to the catch block
if let Some(exception_var) = &catch_block.exception_var {
Expand All @@ -2733,7 +2742,7 @@ impl<'a> TypeChecker<'a> {

// Create a new environment for the finally block
if let Some(finally_statements) = finally_statements {
let finally_env = SymbolEnv::new(Some(env.get_ref()), env.return_type, false, env.phase, stmt.idx);
let finally_env = SymbolEnv::new(Some(env.get_ref()), env.return_type, false, false, env.phase, stmt.idx);
finally_statements.set_env(finally_env);
self.inner_scopes.push(finally_statements);
}
Expand Down Expand Up @@ -2780,6 +2789,7 @@ impl<'a> TypeChecker<'a> {
Some(class_env.get_ref()),
self.types.void(),
true,
true,
class_env.phase,
scope.statements[0].idx,
);
Expand Down Expand Up @@ -2892,6 +2902,7 @@ impl<'a> TypeChecker<'a> {
Some(parent_env.get_ref()),
method_sig.return_type,
is_init,
true,
method_sig.phase,
statement_idx,
);
Expand Down Expand Up @@ -3113,7 +3124,14 @@ impl<'a> TypeChecker<'a> {
types_map.insert(format!("{o}"), (*o, *n));
}

let new_env = SymbolEnv::new(None, original_type_class.env.return_type, false, Phase::Independent, 0);
let new_env = SymbolEnv::new(
None,
original_type_class.env.return_type,
false,
false,
Phase::Independent,
0,
);
let tt = Type::Class(Class {
name: original_type_class.name.clone(),
env: new_env,
Expand Down
13 changes: 8 additions & 5 deletions libs/wingc/src/type_check/jsii_importer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ impl<'a> JsiiImporter<'a> {
} else {
let ns = self.wing_types.add_namespace(Namespace {
name: type_name.assembly().to_string(),
env: SymbolEnv::new(None, self.wing_types.void(), false, Phase::Preflight, 0),
env: SymbolEnv::new(None, self.wing_types.void(), false, false, Phase::Preflight, 0),
loaded: false,
});
self
Expand Down Expand Up @@ -242,7 +242,7 @@ impl<'a> JsiiImporter<'a> {
} else {
let ns = self.wing_types.add_namespace(Namespace {
name: namespace_name.to_string(),
env: SymbolEnv::new(None, self.wing_types.void(), false, Phase::Preflight, 0),
env: SymbolEnv::new(None, self.wing_types.void(), false, false, Phase::Preflight, 0),
loaded: false,
});
parent_ns
Expand Down Expand Up @@ -311,6 +311,7 @@ impl<'a> JsiiImporter<'a> {
None,
self.wing_types.void(),
false,
false,
Phase::Preflight,
self.jsii_spec.import_statement_idx,
);
Expand All @@ -324,6 +325,7 @@ impl<'a> JsiiImporter<'a> {
None,
self.wing_types.void(),
false,
false,
iface_env.phase,
self.jsii_spec.import_statement_idx,
), // Dummy env, will be replaced below
Expand All @@ -336,6 +338,7 @@ impl<'a> JsiiImporter<'a> {
None,
self.wing_types.void(),
false,
false,
iface_env.phase,
self.jsii_spec.import_statement_idx,
), // Dummy env, will be replaced below
Expand Down Expand Up @@ -609,7 +612,7 @@ impl<'a> JsiiImporter<'a> {
};

// Create environment representing this class, for now it'll be empty just so we can support referencing ourselves from the class definition.
let dummy_env = SymbolEnv::new(None, self.wing_types.void(), false, class_phase, 0);
let dummy_env = SymbolEnv::new(None, self.wing_types.void(), false, false, class_phase, 0);
let new_type_symbol = Self::jsii_name_to_symbol(type_name, &jsii_class.location_in_module);
// Create the new resource/class type and add it to the current environment.
// When adding the class methods below we'll be able to reference this type.
Expand Down Expand Up @@ -660,7 +663,7 @@ impl<'a> JsiiImporter<'a> {
self.register_jsii_type(&jsii_class_fqn, &new_type_symbol, new_type);

// Create class's actual environment before we add properties and methods to it
let mut class_env = SymbolEnv::new(base_class_env, self.wing_types.void(), false, class_phase, 0);
let mut class_env = SymbolEnv::new(base_class_env, self.wing_types.void(), false, false, class_phase, 0);

// Add constructor to the class environment
let jsii_initializer = jsii_class.initializer.as_ref();
Expand Down Expand Up @@ -877,7 +880,7 @@ impl<'a> JsiiImporter<'a> {
{
let ns = self.wing_types.add_namespace(Namespace {
name: assembly.name.clone(),
env: SymbolEnv::new(None, self.wing_types.void(), false, Phase::Preflight, 0),
env: SymbolEnv::new(None, self.wing_types.void(), false, false, Phase::Preflight, 0),
loaded: false,
});
self
Expand Down
28 changes: 24 additions & 4 deletions libs/wingc/src/type_check/symbol_env.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub struct SymbolEnv {
pub return_type: TypeRef,

pub is_init: bool,
pub is_function: bool,
pub phase: Phase,
statement_idx: usize,
}
Expand Down Expand Up @@ -155,6 +156,7 @@ impl SymbolEnv {
parent: Option<SymbolEnvRef>,
return_type: TypeRef,
is_init: bool,
is_function: bool,
phase: Phase,
statement_idx: usize,
) -> Self {
Expand All @@ -166,6 +168,7 @@ impl SymbolEnv {
parent,
return_type,
is_init,
is_function,
phase,
statement_idx,
}
Expand Down Expand Up @@ -350,6 +353,14 @@ impl SymbolEnv {
pub fn iter(&self, with_ancestry: bool) -> SymbolEnvIter {
SymbolEnvIter::new(self, with_ancestry)
}

pub fn is_in_function(&self) -> bool {
let mut curr_env = self.get_ref();
while !curr_env.is_function && !curr_env.is_root() {
curr_env = curr_env.parent.unwrap();
}
curr_env.is_function
}
}

pub struct SymbolEnvIter<'a> {
Expand Down Expand Up @@ -419,12 +430,13 @@ mod tests {
#[test]
fn test_statement_idx_lookups() {
let types = setup_types();
let mut parent_env = SymbolEnv::new(None, types.void(), false, Phase::Independent, 0);
let mut parent_env = SymbolEnv::new(None, types.void(), false, false, Phase::Independent, 0);
let child_scope_idx = 10;
let mut child_env = SymbolEnv::new(
Some(parent_env.get_ref()),
types.void(),
false,
false,
crate::ast::Phase::Independent,
child_scope_idx,
);
Expand Down Expand Up @@ -538,24 +550,32 @@ mod tests {
#[test]
fn test_nested_lookups() {
let mut types = setup_types();
let mut parent_env = SymbolEnv::new(None, types.void(), false, Phase::Independent, 0);
let mut parent_env = SymbolEnv::new(None, types.void(), false, false, Phase::Independent, 0);
let child_env = SymbolEnv::new(
Some(parent_env.get_ref()),
types.void(),
false,
false,
crate::ast::Phase::Independent,
0,
);

// Create namespaces
let ns1 = types.add_namespace(Namespace {
name: "ns1".to_string(),
env: SymbolEnv::new(None, types.void(), false, Phase::Independent, 0),
env: SymbolEnv::new(None, types.void(), false, false, Phase::Independent, 0),
loaded: false,
});
let ns2 = types.add_namespace(Namespace {
name: "ns2".to_string(),
env: SymbolEnv::new(Some(ns1.env.get_ref()), types.void(), false, Phase::Independent, 0),
env: SymbolEnv::new(
Some(ns1.env.get_ref()),
types.void(),
false,
false,
Phase::Independent,
0,
),
loaded: false,
});

Expand Down
Loading

0 comments on commit a62650a

Please sign in to comment.