From bf5087994cda59ded4edfd36d5e8055abeafbc1b Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Mon, 30 Aug 2021 01:18:58 +0200 Subject: [PATCH 1/7] instance: Add `set_field` method --- src/instance/mod.rs | 43 +++++++++++++++++++++++++-- src/instruction/type_instantiation.rs | 33 ++++++++------------ 2 files changed, 53 insertions(+), 23 deletions(-) diff --git a/src/instance/mod.rs b/src/instance/mod.rs index 657b131f..d8ed8b87 100644 --- a/src/instance/mod.rs +++ b/src/instance/mod.rs @@ -13,9 +13,9 @@ use crate::instruction::TypeDec; use crate::{ErrKind, Error}; pub type Ty = TypeDec; -pub type Name = String; +type Name = String; type Offset = usize; -pub type Size = usize; +type Size = usize; type FieldsMap = HashMap; /// The type is optional. At first, the type might not be known, and will only be @@ -35,6 +35,10 @@ impl ObjectInstance { ObjectInstance::new(None, 0, vec![], None) } + pub fn empty_with_fields() -> ObjectInstance { + ObjectInstance::new(None, 0, vec![], Some(vec![])) + } + /// Create a new instance pub fn new( ty: Option, @@ -101,6 +105,41 @@ impl ObjectInstance { } } + fn add_field(&mut self, name: &str, value: ObjectInstance) -> Result<(), Error> { + self.size += value.size(); + self.data.append(&mut value.data().to_vec()); + + // We can unwrap safely here since we already checked if the instance contained fields + // in `set_field()` + self.fields.as_mut().unwrap().insert(name.to_string(), (self.size - value.size(), value.size())); + + Ok(()) + } + + fn mutate_field(&mut self, name: &str, value: ObjectInstance) -> Result<(), Error> { + // We can unwrap safely here since we already checked if the instance contained fields + // in `set_field()`, and that the field was present + let (offset, size) = self.fields.as_mut().unwrap().get(name).unwrap(); + + for i in *offset..(offset + size) { + if let Some(data) = self.data.get_mut(offset + i) { + *data = *value.data().get(i).unwrap(); + } + } + + Ok(()) + } + + pub fn set_field(&mut self, field_name: &str, field_value: ObjectInstance) -> Result<(), Error> { + match &mut self.fields { + None => Err(Error::new(ErrKind::Context).with_msg(String::from("no fields on instance"))), + Some(field_map) => match field_map.contains_key(field_name) { + false => self.add_field(field_name, field_value), + true => self.mutate_field(field_name, field_value), + } + } + } + pub fn fields(&self) -> &Option { &self.fields } diff --git a/src/instruction/type_instantiation.rs b/src/instruction/type_instantiation.rs index 2c7023b7..fc8c5fc2 100644 --- a/src/instruction/type_instantiation.rs +++ b/src/instruction/type_instantiation.rs @@ -5,7 +5,6 @@ use super::{ Context, ErrKind, Error, InstrKind, Instruction, ObjectInstance, Rename, TypeDec, TypeId, VarAssign, }; -use crate::instance::{Name, Size}; use std::rc::Rc; @@ -116,31 +115,23 @@ impl Instruction for TypeInstantiation { return None; } - let mut size: usize = 0; - let mut data: Vec = Vec::new(); - let mut fields: Vec<(Name, Size)> = Vec::new(); - for (_, named_arg) in self.fields.iter().enumerate() { - // FIXME: Need to assign the correct field to the field that corresponds - // in the typedec + let mut instance = ObjectInstance::empty_with_fields(); + + for named_arg in self.fields.iter() { let field_instr = named_arg.value(); let field_name = named_arg.symbol(); + let field_instance = field_instr.execute_expression(ctx)?; - // FIXME: Use execute_expression() here? - let instance = field_instr.execute_expression(ctx)?; - - let inst_size = instance.size(); - size += inst_size; - fields.push((field_name.to_string(), inst_size)); - data.append(&mut instance.data().to_vec()); + if let Err(e) = instance.set_field(field_name, field_instance) { + ctx.error(e); + return None; + } } - Some(ObjectInstance::new( - // FIXME: Disgusting, maybe do not use Rc for TypeId? - Some((*type_dec).clone()), - size, - data, - Some(fields), - )) + // FIXME: Disgusting + instance.set_ty(Some((*type_dec).clone())); + + Some(instance) } } From 943f808335041c8f8461f20c3f9bb809c44aa439 Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Mon, 30 Aug 2021 01:23:38 +0200 Subject: [PATCH 2/7] field_assign: Add base parsing and executing logic --- src/instruction/field_access.rs | 4 +-- src/instruction/field_assign.rs | 52 +++++++++++++++++++++++++++++++++ src/instruction/mod.rs | 2 ++ src/parser/box_construct.rs | 1 + src/parser/constructs.rs | 17 +++++++---- 5 files changed, 69 insertions(+), 7 deletions(-) create mode 100644 src/instruction/field_assign.rs diff --git a/src/instruction/field_access.rs b/src/instruction/field_access.rs index 7fe8c8b3..2b0e59a8 100644 --- a/src/instruction/field_access.rs +++ b/src/instruction/field_access.rs @@ -5,8 +5,8 @@ use crate::{Context, ErrKind, Error, InstrKind, Instruction, ObjectInstance, Ren #[derive(Clone)] pub struct FieldAccess { - instance: Box, - field_name: String, + pub(crate) instance: Box, + pub(crate) field_name: String, } impl FieldAccess { diff --git a/src/instruction/field_assign.rs b/src/instruction/field_assign.rs new file mode 100644 index 00000000..3f142137 --- /dev/null +++ b/src/instruction/field_assign.rs @@ -0,0 +1,52 @@ +//! FieldAssigns represent the assignment of a value to a given field on an instance +//! This is what is used by the interpreter when modifying an attribute on a given type. +//! Just like variable assignments, the original instance needs to be mutable in order to +//! be assigned a new value. However, unlike variable assignments, there is no "first +//! assignment" for fields, as they should be initialized on the type's instantiation. + +use crate::{Context, InstrKind, Instruction, ObjectInstance, Rename}; +use super::FieldAccess; + +#[derive(Clone)] +pub struct FieldAssign { + field: FieldAccess, + value: Box, +} + +impl FieldAssign { + /// Create a new FieldAssign from the FieldAccess you're trying to modify and the value + /// you're trying to assign to it. + pub fn new(field: FieldAccess, value: Box) -> FieldAssign { + FieldAssign { + field, value + } + } +} + +impl Rename for FieldAssign { + fn prefix(&mut self, _prefix: &str) { + /* FIXME: Add logic */ + } +} + +impl Instruction for FieldAssign { + fn kind(&self) -> InstrKind { + InstrKind::Statement + } + + fn print(&self) -> String { + format!("{} = {}", self.field.print(), self.value.print()) + } + + fn execute(&self, ctx: &mut Context) -> Option { + let mut instance = self.field.execute(ctx)?; + + // FIXME: Should we check if this is the first time we're setting the field? In + // that case, error out! + if let Err(e) = instance.set_field(&self.field.field_name, self.value.execute(ctx)?) { + ctx.error(e); + } + + None + } +} diff --git a/src/instruction/mod.rs b/src/instruction/mod.rs index 4f095954..2fe3b53d 100644 --- a/src/instruction/mod.rs +++ b/src/instruction/mod.rs @@ -12,6 +12,7 @@ mod block; mod dec_arg; mod extra_content; mod field_access; +mod field_assign; mod function_call; mod function_declaration; mod if_else; @@ -32,6 +33,7 @@ pub use block::Block; pub use dec_arg::DecArg; pub use extra_content::{CommentKind, ExtraContent, ExtraKind}; pub use field_access::FieldAccess; +pub use field_assign::FieldAssign; pub use function_call::FunctionCall; pub use function_declaration::{FunctionDec, FunctionKind}; pub use if_else::IfElse; diff --git a/src/parser/box_construct.rs b/src/parser/box_construct.rs index a26d0272..b51f384d 100644 --- a/src/parser/box_construct.rs +++ b/src/parser/box_construct.rs @@ -54,5 +54,6 @@ impl BoxConstruct { box_construct! {incl} box_construct! {method_call} box_construct! {field_access} + box_construct! {field_assign} box_construct! {extra} } diff --git a/src/parser/constructs.rs b/src/parser/constructs.rs index 82a1381d..019b8d1c 100644 --- a/src/parser/constructs.rs +++ b/src/parser/constructs.rs @@ -17,11 +17,7 @@ use nom::Err::Error as NomError; use nom::{branch::alt, combinator::opt, multi::many0}; use crate::error::{ErrKind, Error}; -use crate::instruction::{ - Block, DecArg, ExtraContent, FieldAccess, FunctionCall, FunctionDec, FunctionKind, IfElse, - Incl, Instruction, JkInst, Loop, LoopKind, MethodCall, TypeDec, TypeId, TypeInstantiation, Var, - VarAssign, -}; +use crate::instruction::{Block, DecArg, ExtraContent, FieldAccess, FieldAssign, FunctionCall, FunctionDec, FunctionKind, IfElse, Incl, Instruction, JkInst, Loop, LoopKind, MethodCall, TypeDec, TypeId, TypeInstantiation, Var, VarAssign}; use crate::parser::{BoxConstruct, ConstantConstruct, ParseResult, ShuntingYard, Token}; type Instructions = Vec>; @@ -39,6 +35,7 @@ impl Construct { let (input, value) = alt(( Construct::binary_op, BoxConstruct::method_call, + BoxConstruct::field_assign, BoxConstruct::field_access, BoxConstruct::function_declaration, BoxConstruct::type_declaration, @@ -879,6 +876,16 @@ impl Construct { Ok((input, current_fa)) } + pub fn field_assign(input: &str) -> ParseResult<&str, FieldAssign> { + let (input, field_access) = Construct::field_access(input)?; + let (input, _) = Token::maybe_consume_extra(input)?; + let (input, _) = Token::equal(input)?; + let (input, _) = Token::maybe_consume_extra(input)?; + let (input, value) = Construct::instruction(input)?; + + Ok((input, FieldAssign::new(field_access, value))) + } + /// Parse a field access on a custom type. This is very similar to a method call: The /// only difference is that the method call shall have parentheses /// From 5e61d2f6845d20324a0d93c8cc440d3d79fd4534 Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Tue, 31 Aug 2021 18:48:59 +0200 Subject: [PATCH 3/7] field_assign: WIP: Add test --- src/instance/mod.rs | 17 ++++++++++--- src/instruction/field_assign.rs | 44 ++++++++++++++++++++++++++++++--- src/parser/constructs.rs | 6 ++++- 3 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/instance/mod.rs b/src/instance/mod.rs index d8ed8b87..6cf26d08 100644 --- a/src/instance/mod.rs +++ b/src/instance/mod.rs @@ -111,7 +111,10 @@ impl ObjectInstance { // We can unwrap safely here since we already checked if the instance contained fields // in `set_field()` - self.fields.as_mut().unwrap().insert(name.to_string(), (self.size - value.size(), value.size())); + self.fields + .as_mut() + .unwrap() + .insert(name.to_string(), (self.size - value.size(), value.size())); Ok(()) } @@ -130,13 +133,19 @@ impl ObjectInstance { Ok(()) } - pub fn set_field(&mut self, field_name: &str, field_value: ObjectInstance) -> Result<(), Error> { + pub fn set_field( + &mut self, + field_name: &str, + field_value: ObjectInstance, + ) -> Result<(), Error> { match &mut self.fields { - None => Err(Error::new(ErrKind::Context).with_msg(String::from("no fields on instance"))), + None => { + Err(Error::new(ErrKind::Context).with_msg(String::from("no fields on instance"))) + } Some(field_map) => match field_map.contains_key(field_name) { false => self.add_field(field_name, field_value), true => self.mutate_field(field_name, field_value), - } + }, } } diff --git a/src/instruction/field_assign.rs b/src/instruction/field_assign.rs index 3f142137..e7b3e0c9 100644 --- a/src/instruction/field_assign.rs +++ b/src/instruction/field_assign.rs @@ -4,8 +4,8 @@ //! be assigned a new value. However, unlike variable assignments, there is no "first //! assignment" for fields, as they should be initialized on the type's instantiation. -use crate::{Context, InstrKind, Instruction, ObjectInstance, Rename}; use super::FieldAccess; +use crate::{Context, InstrKind, Instruction, ObjectInstance, Rename}; #[derive(Clone)] pub struct FieldAssign { @@ -17,9 +17,7 @@ impl FieldAssign { /// Create a new FieldAssign from the FieldAccess you're trying to modify and the value /// you're trying to assign to it. pub fn new(field: FieldAccess, value: Box) -> FieldAssign { - FieldAssign { - field, value - } + FieldAssign { field, value } } } @@ -50,3 +48,41 @@ impl Instruction for FieldAssign { None } } + +#[cfg(test)] +mod tests { + use crate::{parser::Construct, Context, JkInt}; + use crate::instance::ToObjectInstance; + + fn setup() -> Context { + let mut ctx = Context::new(); + + let inst = Construct::instruction("type Point(x: int, y: int);") + .unwrap() + .1; + inst.execute(&mut ctx); + + let inst = Construct::instruction("point = Point { x = 15, y = 14 }") + .unwrap() + .1; + inst.execute(&mut ctx); + + assert!(!ctx.error_handler.has_errors()); + + ctx + } + + #[test] + fn t_valid_field_assign() { + let mut ctx = setup(); + + let f_a = Construct::instruction("point.x = 99").unwrap().1; + f_a.execute(&mut ctx); + + let f_a_result = Construct::instruction("point.x").unwrap().1; + let x_value = f_a_result.execute(&mut ctx).unwrap(); + + assert!(!ctx.error_handler.has_errors()); + assert_eq!(x_value, JkInt::from(99).to_instance()); + } +} diff --git a/src/parser/constructs.rs b/src/parser/constructs.rs index 019b8d1c..4d6c64b7 100644 --- a/src/parser/constructs.rs +++ b/src/parser/constructs.rs @@ -17,7 +17,11 @@ use nom::Err::Error as NomError; use nom::{branch::alt, combinator::opt, multi::many0}; use crate::error::{ErrKind, Error}; -use crate::instruction::{Block, DecArg, ExtraContent, FieldAccess, FieldAssign, FunctionCall, FunctionDec, FunctionKind, IfElse, Incl, Instruction, JkInst, Loop, LoopKind, MethodCall, TypeDec, TypeId, TypeInstantiation, Var, VarAssign}; +use crate::instruction::{ + Block, DecArg, ExtraContent, FieldAccess, FieldAssign, FunctionCall, FunctionDec, FunctionKind, + IfElse, Incl, Instruction, JkInst, Loop, LoopKind, MethodCall, TypeDec, TypeId, + TypeInstantiation, Var, VarAssign, +}; use crate::parser::{BoxConstruct, ConstantConstruct, ParseResult, ShuntingYard, Token}; type Instructions = Vec>; From c6ae996f4477ea90ce26d72ebfb9db907eb288d1 Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Tue, 31 Aug 2021 21:02:49 +0200 Subject: [PATCH 4/7] context: WIP: Make get_variable() mutable --- src/context.rs | 3 ++- src/context/scope_map.rs | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/context.rs b/src/context.rs index 534c0f70..67e20140 100644 --- a/src/context.rs +++ b/src/context.rs @@ -172,7 +172,8 @@ impl Context { } /// Get a reference on an existing variable - pub fn get_variable(&self, name: &str) -> Option<&Var> { + // Should we get_mut or should we replace the variable?? + pub fn get_variable(&mut self, name: &str) -> Option<&mut Var> { self.scope_map.get_variable(name) } diff --git a/src/context/scope_map.rs b/src/context/scope_map.rs index 5ee0cdb5..b96b86e8 100644 --- a/src/context/scope_map.rs +++ b/src/context/scope_map.rs @@ -29,8 +29,8 @@ impl Scope { } /// Get a reference on a variable from the scope map if is has been inserted already - pub fn get_variable(&self, name: &str) -> Option<&Var> { - self.variables.get(name) + pub fn get_variable(&mut self, name: &str) -> Option<&mut Var> { + self.variables.get_mut(name) } /// Get a reference on a function from the scope map if is has been inserted already @@ -139,9 +139,9 @@ impl ScopeMap { } /// Maybe get a variable in any available scopes - pub fn get_variable(&self, name: &str) -> Option<&Var> { + pub fn get_variable(&mut self, name: &str) -> Option<&mut Var> { // FIXME: Use find for code quality? - for scope in self.scopes.iter() { + for scope in self.scopes.iter_mut() { match scope.get_variable(name) { Some(v) => return Some(v), None => continue, @@ -245,7 +245,7 @@ mod tests { #[test] fn t_find_non_existent_var() { - let s = ScopeMap::new(); + let mut s = ScopeMap::new(); assert!(s.get_variable("a").is_none()); } From 4d9ed42d19a6a7cc1517a8071bf4f03e53622b1d Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Tue, 31 Aug 2021 21:03:08 +0200 Subject: [PATCH 5/7] field_assign: WIP: Field access almost possible --- src/instruction/field_assign.rs | 27 +++++++++++++++++++++++---- src/instruction/var.rs | 12 ++++-------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/instruction/field_assign.rs b/src/instruction/field_assign.rs index e7b3e0c9..3d9c43c5 100644 --- a/src/instruction/field_assign.rs +++ b/src/instruction/field_assign.rs @@ -5,10 +5,11 @@ //! assignment" for fields, as they should be initialized on the type's instantiation. use super::FieldAccess; -use crate::{Context, InstrKind, Instruction, ObjectInstance, Rename}; +use crate::{Context, Error, ErrKind, InstrKind, Instruction, ObjectInstance, Rename}; #[derive(Clone)] pub struct FieldAssign { + // FIXME: Figure out how to keep a field access or a variable here field: FieldAccess, value: Box, } @@ -37,11 +38,26 @@ impl Instruction for FieldAssign { } fn execute(&self, ctx: &mut Context) -> Option { - let mut instance = self.field.execute(ctx)?; + // FIXME: We probably need to keep track of all the instances created somewhere + // in the context, to garbage collect them later for exemple + let value = self.value.execute(ctx)?; + + // FIXME: How does this work when we're not dealing with a variable as an instance? + // Say, `fn_call().attribute = some_other_value` (Hint: It doesn't) + let instance = match ctx.get_variable(&self.field.instance.print()) { + Some(var) => &mut var.instance, + None => { + ctx.error(Error::new(ErrKind::Context).with_msg(format!( + "cannot find variable: `{}`", + self.field.instance.print() + ))); + return None; + } + }; // FIXME: Should we check if this is the first time we're setting the field? In // that case, error out! - if let Err(e) = instance.set_field(&self.field.field_name, self.value.execute(ctx)?) { + if let Err(e) = instance.set_field(&self.field.field_name, value) { ctx.error(e); } @@ -51,8 +67,8 @@ impl Instruction for FieldAssign { #[cfg(test)] mod tests { - use crate::{parser::Construct, Context, JkInt}; use crate::instance::ToObjectInstance; + use crate::{parser::Construct, Context, JkInt}; fn setup() -> Context { let mut ctx = Context::new(); @@ -85,4 +101,7 @@ mod tests { assert!(!ctx.error_handler.has_errors()); assert_eq!(x_value, JkInt::from(99).to_instance()); } + + // FIXME: Add tests making sure that we can't modify the fields on something that + // isn't a variable } diff --git a/src/instruction/var.rs b/src/instruction/var.rs index 369e6de4..05bfcc3b 100644 --- a/src/instruction/var.rs +++ b/src/instruction/var.rs @@ -10,7 +10,7 @@ use crate::{Context, ErrKind, Error, InstrKind, Instruction, JkBool, ObjectInsta pub struct Var { name: String, mutable: bool, - instance: ObjectInstance, + pub(crate) instance: ObjectInstance, } impl Var { @@ -28,11 +28,6 @@ impl Var { &self.name } - /// Return a copy of the variable's instance - pub fn instance(&self) -> ObjectInstance { - self.instance.clone() - } - /// Is a variable mutable or not pub fn mutable(&self) -> bool { self.mutable @@ -110,9 +105,10 @@ impl Instruction for Var { } }; - ctx.debug("VAR", var.print().as_ref()); + // FIXME: Re-add once debugging is separate from context #210 + // ctx.debug("VAR", var.print().as_ref()); - Some(var.instance()) + Some(var.instance.clone()) } } From 943f70baba0a10f4b649311f1d075bedc1ccdd96 Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Tue, 31 Aug 2021 21:29:41 +0200 Subject: [PATCH 6/7] instruction: Add FIXME for later --- src/instruction/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/instruction/mod.rs b/src/instruction/mod.rs index 2fe3b53d..498bc248 100644 --- a/src/instruction/mod.rs +++ b/src/instruction/mod.rs @@ -67,6 +67,7 @@ pub enum InstrKind { pub trait Instruction: InstructionClone + Downcast + Rename { /// Execute the instruction, altering the state of the context. Executing /// this method may return an object instance + // FIXME: Should this return a mutable ref instead?? On an instance kept in the context? fn execute(&self, _ctx: &mut Context) -> Option { unreachable!( "\n{}\n --> {}", From b4fbfb97bd7bf5275377e66030f17d9f8b01078a Mon Sep 17 00:00:00 2001 From: CohenArthur Date: Mon, 6 Sep 2021 00:55:37 +0200 Subject: [PATCH 7/7] field_assign: Add comment to figure out how to assign a new value --- src/instruction/field_assign.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/instruction/field_assign.rs b/src/instruction/field_assign.rs index 3d9c43c5..2169a0ea 100644 --- a/src/instruction/field_assign.rs +++ b/src/instruction/field_assign.rs @@ -9,7 +9,8 @@ use crate::{Context, Error, ErrKind, InstrKind, Instruction, ObjectInstance, Ren #[derive(Clone)] pub struct FieldAssign { - // FIXME: Figure out how to keep a field access or a variable here + // FIXME: This should actually be a variable and be kinda similar to VarAssign + // in that regard field: FieldAccess, value: Box, }