Remove RefCell from streams (#227)

Cargo.lock

@@ -13,7 +13,7 @@ dependencies = [
 
 [[package]]
 name = "air"
-version = "0.20.0"
+version = "0.21.0"
 dependencies = [
  "air-execution-info-collector",
  "air-interpreter-data",
@@ -51,7 +51,7 @@ version = "0.1.0"
 
 [[package]]
 name = "air-interpreter"
-version = "0.20.0"
+version = "0.21.0"
 dependencies = [
  "air",
  "air-log-targets",

air/src/execution_step/air/ap/utils.rs

@@ -81,7 +81,7 @@ fn variable_to_generations(variable: &ast::Variable<'_>, exec_ctx: &ExecutionCtx
             // unwrap here is safe because this function will be called only
             // when this stream's been created
             let stream = exec_ctx.streams.get(stream.name, stream.position).unwrap();
-            let generation = match stream.borrow().generations_count() {
+            let generation = match stream.generations_count() {
                 0 => 0,
                 n => n - 1,
             };

air/src/execution_step/air/call/call_result_setter.rs

@@ -42,7 +42,7 @@ pub(crate) fn set_local_result<'i>(
             // TODO: refactor this generation handling
             let generation = match exec_ctx.streams.get(stream.name, stream.position) {
                 Some(stream) => {
-                    let generation = match stream.borrow().generations_count() {
+                    let generation = match stream.generations_count() {
                         0 => 0,
                         n => n - 1,
                     };

air/src/execution_step/air/fold/utils.rs

@@ -22,7 +22,6 @@ use crate::SecurityTetraplet;
 
 use air_parser::ast;
 
-use std::cell::RefCell;
 use std::ops::Deref;
 use std::rc::Rc;
 
@@ -48,11 +47,10 @@ pub(crate) fn construct_scalar_iterable_value<'ctx>(
 
 /// Constructs iterable value for given stream iterable.
 pub(crate) fn construct_stream_iterable_values(
-    stream: &RefCell<Stream>,
+    stream: &Stream,
     start: Generation,
     end: Generation,
 ) -> Vec<IterableValue> {
-    let stream = stream.borrow();
     let stream_iter = match stream.slice_iter(start, end) {
         Some(stream_iter) => stream_iter,
         None => return vec![],

air/src/execution_step/air/fold_stream.rs

@@ -25,13 +25,11 @@ use super::TraceHandler;
 use crate::execution_step::boxed_value::Stream;
 use crate::log_instruction;
 use crate::trace_to_exec_err;
-use air_parser::ast;
 use stream_cursor::StreamCursor;
 
+use air_parser::ast;
 use air_parser::ast::FoldStream;
 
-use std::cell::RefCell;
-
 impl<'i> ExecutableInstruction<'i> for FoldStream<'i> {
     fn execute(&self, exec_ctx: &mut ExecutionCtx<'i>, trace_ctx: &mut TraceHandler) -> ExecutionResult<()> {
         log_instruction!(fold, exec_ctx, trace_ctx);
@@ -118,16 +116,16 @@ fn should_stop_iteration(iteration_result: &ExecutionResult<bool>) -> bool {
 
 /// Safety: this function should be called iff stream is present in context
 fn add_new_generation_if_non_empty(stream: &ast::Stream<'_>, exec_ctx: &mut ExecutionCtx<'_>) {
-    let stream = exec_ctx.streams.get(stream.name, stream.position).unwrap();
+    let stream = exec_ctx.streams.get_mut(stream.name, stream.position).unwrap();
-    stream.borrow_mut().add_new_generation_if_non_empty();
+    stream.add_new_generation_if_non_empty();
 }
 
 /// Safety: this function should be called iff stream is present in context
 fn remove_new_generation_if_non_empty<'ctx>(
     stream: &ast::Stream<'_>,
     exec_ctx: &'ctx mut ExecutionCtx<'_>,
-) -> &'ctx RefCell<Stream> {
+) -> &'ctx Stream {
-    let stream = exec_ctx.streams.get(stream.name, stream.position).unwrap();
+    let stream = exec_ctx.streams.get_mut(stream.name, stream.position).unwrap();
-    stream.borrow_mut().remove_last_generation_if_empty();
+    stream.remove_last_generation_if_empty();
     stream
 }

air/src/execution_step/air/fold_stream/stream_cursor.rs

@@ -19,8 +19,6 @@ use crate::execution_step::air::fold::IterableValue;
 use crate::execution_step::boxed_value::Generation;
 use crate::execution_step::boxed_value::Stream;
 
-use std::cell::RefCell;
-
 pub(super) struct StreamCursor {
     last_seen_generation: u32,
 }
@@ -32,10 +30,10 @@ impl StreamCursor {
         }
     }
 
-    pub(super) fn construct_iterables(&mut self, stream: &RefCell<Stream>) -> Vec<IterableValue> {
+    pub(super) fn construct_iterables(&mut self, stream: &Stream) -> Vec<IterableValue> {
         let iterables =
             construct_stream_iterable_values(stream, Generation::Nth(self.last_seen_generation), Generation::Last);
-        self.last_seen_generation = stream.borrow().non_empty_generations_count() as u32;
+        self.last_seen_generation = stream.non_empty_generations_count() as u32;
 
         iterables
     }

air/src/execution_step/boxed_value/jvaluable/stream.rs

@@ -32,7 +32,7 @@ use std::ops::Deref;
 
 #[derive(Debug)]
 pub(crate) struct StreamJvaluableIngredients<'stream> {
-    pub(crate) stream: std::cell::Ref<'stream, Stream>,
+    pub(crate) stream: &'stream Stream,
     pub(crate) generation: Generation,
 }
 
@@ -63,7 +63,7 @@ impl JValuable for StreamJvaluableIngredients<'_> {
     }
 
     fn as_jvalue(&self) -> Cow<'_, JValue> {
-        let jvalue = self.stream.deref().clone().as_jvalue(self.generation).unwrap();
+        let jvalue = self.stream.as_jvalue(self.generation).unwrap();
         Cow::Owned(jvalue)
     }
 
@@ -83,7 +83,7 @@ impl JValuable for StreamJvaluableIngredients<'_> {
 use crate::execution_step::boxed_value::StreamIter;
 
 impl<'stream> StreamJvaluableIngredients<'stream> {
-    pub(crate) fn new(stream: std::cell::Ref<'stream, Stream>, generation: Generation) -> Self {
+    pub(crate) fn new(stream: &'stream Stream, generation: Generation) -> Self {
         Self { stream, generation }
     }
 
@@ -98,7 +98,7 @@ impl<'stream> StreamJvaluableIngredients<'stream> {
                     Generation::Last => unreachable!(),
                 };
 
-                Err(StreamDontHaveSuchGeneration(self.stream.deref().clone(), generation as usize).into())
+                Err(StreamDontHaveSuchGeneration(self.stream.clone(), generation as usize).into())
             }
         }
     }

air/src/execution_step/execution_context/streams_variables.rs

@@ -22,7 +22,6 @@ use crate::execution_step::ValueAggregate;
 use air_interpreter_data::GlobalStreamGens;
 use air_interpreter_data::RestrictedStreamGens;
 
-use std::cell::RefCell;
 use std::collections::hash_map::Entry::{Occupied, Vacant};
 use std::collections::HashMap;
 
@@ -43,17 +42,22 @@ pub(crate) struct Streams {
 
 struct StreamDescriptor {
     pub(self) span: Span,
-    // TODO: get rid of RefCell in a separate PR
+    pub(self) stream: Stream,
-    pub(self) stream: RefCell<Stream>,
 }
 
 impl Streams {
-    pub(crate) fn get(&self, name: &str, position: usize) -> Option<&RefCell<Stream>> {
+    pub(crate) fn get(&self, name: &str, position: usize) -> Option<&Stream> {
         self.streams
             .get(name)
             .and_then(|descriptors| find_closest(descriptors.iter(), position))
     }
 
+    pub(crate) fn get_mut(&mut self, name: &str, position: usize) -> Option<&mut Stream> {
+        self.streams
+            .get_mut(name)
+            .and_then(|descriptors| find_closest_mut(descriptors.iter_mut(), position))
+    }
+
     pub(crate) fn add_stream_value(
         &mut self,
         value: ValueAggregate,
@@ -61,8 +65,8 @@ impl Streams {
         stream_name: &str,
         position: usize,
     ) -> ExecutionResult<u32> {
-        match self.get(stream_name, position) {
+        match self.get_mut(stream_name, position) {
-            Some(stream) => stream.borrow_mut().add_value(value, generation),
+            Some(stream) => stream.add_value(value, generation),
             None => {
                 // streams could be created in three ways:
                 //  - after met new instruction with stream name that isn't present in streams
@@ -80,7 +84,7 @@ impl Streams {
     }
 
     pub(crate) fn add_global_stream(&mut self, name: String, stream: Stream) {
-        let descriptor = StreamDescriptor::global(RefCell::new(stream));
+        let descriptor = StreamDescriptor::global(stream);
         self.streams.insert(name, vec![descriptor]);
     }
 
@@ -90,7 +94,7 @@ impl Streams {
             .stream_generation_from_data(&name, span.left as u32, iteration as usize)
             .unwrap_or_default();
 
-        let new_stream = RefCell::new(Stream::from_generations_count(generations_count as usize));
+        let new_stream = Stream::from_generations_count(generations_count as usize);
         let new_descriptor = StreamDescriptor::restricted(new_stream, span);
         match self.streams.entry(name) {
             Occupied(mut entry) => {
@@ -112,11 +116,7 @@ impl Streams {
             self.streams.remove(&name);
         }
 
-        self.collect_stream_generation(
+        self.collect_stream_generation(name, position, last_descriptor.stream.generations_count() as u32);
-            name,
-            position,
-            last_descriptor.stream.borrow().generations_count() as u32,
-        );
     }
 
     /// This method must be called at the end of execution, because it contains logic to collect
@@ -131,7 +131,7 @@ impl Streams {
                 // unwrap is safe here because of invariant that streams contains non-empty vectors,
                 // moreover it must contain only one value, because this method is called at the end
                 // of the execution
-                let generation = descriptors.pop().unwrap().stream.borrow().generations_count();
+                let generation = descriptors.pop().unwrap().stream.generations_count();
                 (name, generation as u32)
             })
             .collect::<GlobalStreamGens>();
@@ -165,14 +165,14 @@ impl Streams {
 }
 
 impl StreamDescriptor {
-    pub(self) fn global(stream: RefCell<Stream>) -> Self {
+    pub(self) fn global(stream: Stream) -> Self {
         Self {
             span: Span::new(0, usize::MAX),
             stream,
         }
     }
 
-    pub(self) fn restricted(stream: RefCell<Stream>, span: Span) -> Self {
+    pub(self) fn restricted(stream: Stream, span: Span) -> Self {
         Self { span, stream }
     }
 }
@@ -180,7 +180,7 @@ impl StreamDescriptor {
 fn find_closest<'d>(
     descriptors: impl DoubleEndedIterator<Item = &'d StreamDescriptor>,
     position: usize,
-) -> Option<&'d RefCell<Stream>> {
+) -> Option<&'d Stream> {
     // descriptors are placed in a order of decreasing scopes, so it's enough to get the latest suitable
     for descriptor in descriptors.rev() {
         if descriptor.span.contains_position(position) {
@@ -191,6 +191,20 @@ fn find_closest<'d>(
     None
 }
 
+fn find_closest_mut<'d>(
+    descriptors: impl DoubleEndedIterator<Item = &'d mut StreamDescriptor>,
+    position: usize,
+) -> Option<&'d mut Stream> {
+    // descriptors are placed in a order of decreasing scopes, so it's enough to get the latest suitable
+    for descriptor in descriptors.rev() {
+        if descriptor.span.contains_position(position) {
+            return Some(&mut descriptor.stream);
+        }
+    }
+
+    None
+}
+
 use air_parser::ast::Span;
 use std::fmt;
 
@@ -207,12 +221,6 @@ impl fmt::Display for Streams {
 
 impl fmt::Display for StreamDescriptor {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(
+        write!(f, " <{}> - <{}>: {}", self.span.left, self.span.right, self.stream)
-            f,
-            " <{}> - <{}>: {}",
-            self.span.left,
-            self.span.right,
-            self.stream.borrow()
-        )
     }
 }

air/src/execution_step/resolver/resolve.rs

@@ -100,7 +100,7 @@ pub(crate) fn resolve_variable<'ctx, 'i>(
         } => {
             match ctx.streams.get(name, position) {
                 Some(stream) => {
-                    let ingredients = StreamJvaluableIngredients::new(stream.borrow(), generation);
+                    let ingredients = StreamJvaluableIngredients::new(stream, generation);
                     Ok(Box::new(ingredients))
                 }
                 // return an empty stream for not found stream