Skip to content

Commit

Permalink
fix: forbid generic call with mutable vars
Browse files Browse the repository at this point in the history
  • Loading branch information
katat committed Oct 28, 2024
1 parent bde1e37 commit 720db06
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 6 deletions.
80 changes: 80 additions & 0 deletions src/negative_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,58 @@ fn test_generic_const_for_loop() {
));
}

#[test]
fn test_generic_const_mut_for_loop() {
let code = r#"
// generic on const argument
fn gen(const LEN: Field) -> [Field; LEN] {
return [0; LEN];
}
fn loop() {
let mut size = 2;
for ii in 0..3 {
gen(size);
}
}
"#;

let res = tast_pass(code).0;

assert!(matches!(
res.unwrap_err().kind,
ErrorKind::VarAccessForbiddenInForLoop(..)
));
}

#[test]
fn test_generic_mut_struct_for_loop() {
let code = r#"
struct Thing {
xx: Field,
}
// generic on const argument
fn gen(const LEN: Field) -> [Field; LEN] {
return [0; LEN];
}
fn loop() {
let mut thing = Thing {xx: 3};
for ii in 0..3 {
gen(thing.xx);
}
}
"#;

let res = tast_pass(code).0;

assert!(matches!(
res.unwrap_err().kind,
ErrorKind::VarAccessForbiddenInForLoop(..)
));
}

#[test]
fn test_generic_const_nested_for_loop() {
let code = r#"
Expand Down Expand Up @@ -201,6 +253,34 @@ fn test_generic_method_cst_for_loop() {
));
}

#[test]
fn test_generic_struct_self_for_loop() {
let code = r#"
struct Thing {
xx: Field,
}
// generic on const argument
fn Thing.gen(self, const LEN: Field) -> [Field; LEN] {
return [self.xx; LEN];
}
fn loop() {
let thing = Thing { xx: 3 };
for ii in 0..3 {
thing.gen(ii);
}
}
"#;

let res = tast_pass(code).0;

assert!(matches!(
res.unwrap_err().kind,
ErrorKind::VarAccessForbiddenInForLoop(..)
));
}

#[test]
fn test_generic_method_array_for_loop() {
let code = r#"
Expand Down
10 changes: 7 additions & 3 deletions src/type_checker/checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,13 @@ impl<B: Backend> TypeChecker<B> {

// check if generic is allowed
if method_type.sig.require_monomorphization() && typed_fn_env.is_in_forloop() {
for (observed_arg, expected_arg) in
args.iter().zip(method_type.sig.arguments.iter())
{
for (observed_arg, expected_arg) in args.iter().zip(
method_type
.sig
.arguments
.iter()
.filter(|arg| arg.name.value != "self"),
) {
// check if the arg involves generic vars
if !expected_arg.extract_generic_names().is_empty() {
let mut forbidden_env = typed_fn_env.clone();
Expand Down
19 changes: 16 additions & 3 deletions src/type_checker/fn_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,27 @@ impl TypedFnEnv {
self.current_scope >= prefix_scope
}

pub fn is_forbidden(&self, scope: usize) -> bool {
/// Since currently we don't support unrolling, the generic function calls are assumed to target a same instance.
/// This assumption requires a few type checking rules to forbid the cases that needs unrolling.
/// Forbid rules:
/// - Access to variables within the for loop scope.
/// - Access to mutable variables, except it is an array.
/// Because once the array is declared, the size is fixed even if the array is mutable,
/// so the generic value resolved from array size will be same for generic function argument.
pub fn is_forbidden(&self, scope: usize, ty_info: TypeInfo) -> bool {
let in_forbidden_scope = if let Some(forloop_scope) = self.forloop_scopes.first() {
scope >= *forloop_scope
} else {
false
};

self.forbid_forloop_scope && in_forbidden_scope
let forbidden_mutable = ty_info.mutable
&& !matches!(
ty_info.typ,
TyKind::GenericSizedArray(..) | TyKind::Array(..)
);

self.forbid_forloop_scope && (in_forbidden_scope || forbidden_mutable)
}

/// Stores type information about a local variable.
Expand Down Expand Up @@ -151,7 +164,7 @@ impl TypedFnEnv {
// TODO: return an error no?
pub fn get_type_info(&self, ident: &str) -> Result<Option<&TypeInfo>> {
if let Some((scope, type_info)) = self.vars.get(ident) {
if self.is_forbidden(*scope) {
if self.is_forbidden(*scope, type_info.clone()) {
return Err(Error::new(
"type-checker",
ErrorKind::VarAccessForbiddenInForLoop(ident.to_string()),
Expand Down

0 comments on commit 720db06

Please sign in to comment.