From 22e76f1a679eca1f5e586cd132ea3800aa54b69b Mon Sep 17 00:00:00 2001
From: Noah <33094578+coolreader18@users.noreply.github.com>
Date: Mon, 30 Nov 2020 22:35:02 -0600
Subject: [PATCH] Use a boxed slice for codeobject fields

---
 bytecode/src/bytecode.rs    | 45 +++++++++++++-------------
 compiler/src/compile.rs     | 63 +++++++++++++++++++++++--------------
 vm/src/builtins/function.rs |  2 +-
 3 files changed, 62 insertions(+), 48 deletions(-)

diff --git a/bytecode/src/bytecode.rs b/bytecode/src/bytecode.rs
index 85ec43a99..778455581 100644
--- a/bytecode/src/bytecode.rs
+++ b/bytecode/src/bytecode.rs
@@ -93,8 +93,8 @@ impl ConstantBag for BasicBag {
 /// a codeobject. Also a module has a codeobject.
 #[derive(Clone, PartialEq, Serialize, Deserialize)]
 pub struct CodeObject<C: Constant = ConstantData> {
-    pub instructions: Vec<Instruction>,
-    pub locations: Vec<Location>,
+    pub instructions: Box<[Instruction]>,
+    pub locations: Box<[Location]>,
     pub flags: CodeFlags,
     pub posonlyarg_count: usize, // Number of positional-only arguments
     pub arg_count: usize,
@@ -103,15 +103,15 @@ pub struct CodeObject<C: Constant = ConstantData> {
     pub first_line_number: usize,
     pub obj_name: String, // Name of the object that created this code object
     pub cell2arg: Option<Box<[isize]>>,
-    pub constants: Vec<C>,
+    pub constants: Box<[C]>,
     #[serde(bound(
         deserialize = "C::Name: serde::Deserialize<'de>",
         serialize = "C::Name: serde::Serialize"
     ))]
-    pub names: Vec<C::Name>,
-    pub varnames: Vec<C::Name>,
-    pub cellvars: Vec<C::Name>,
-    pub freevars: Vec<C::Name>,
+    pub names: Box<[C::Name]>,
+    pub varnames: Box<[C::Name]>,
+    pub cellvars: Box<[C::Name]>,
+    pub freevars: Box<[C::Name]>,
 }
 
 bitflags! {
@@ -129,12 +129,6 @@ bitflags! {
     }
 }
 
-impl Default for CodeFlags {
-    fn default() -> Self {
-        Self::NEW_LOCALS
-    }
-}
-
 impl CodeFlags {
     pub const NAME_MAPPING: &'static [(&'static str, CodeFlags)] = &[
         ("GENERATOR", CodeFlags::IS_GENERATOR),
@@ -542,8 +536,8 @@ impl<C: Constant> CodeObject<C> {
         obj_name: String,
     ) -> Self {
         CodeObject {
-            instructions: Vec::new(),
-            locations: Vec::new(),
+            instructions: Box::new([]),
+            locations: Box::new([]),
             flags,
             posonlyarg_count,
             arg_count,
@@ -552,11 +546,11 @@ impl<C: Constant> CodeObject<C> {
             first_line_number,
             obj_name,
             cell2arg: None,
-            constants: Vec::new(),
-            names: Vec::new(),
-            varnames: Vec::new(),
-            cellvars: Vec::new(),
-            freevars: Vec::new(),
+            constants: Box::new([]),
+            names: Box::new([]),
+            varnames: Box::new([]),
+            cellvars: Box::new([]),
+            freevars: Box::new([]),
         }
     }
 
@@ -593,7 +587,7 @@ impl<C: Constant> CodeObject<C> {
 
     pub fn label_targets(&self) -> BTreeSet<Label> {
         let mut label_targets = BTreeSet::new();
-        for instruction in &self.instructions {
+        for instruction in &*self.instructions {
             match instruction {
                 Jump { target: l }
                 | JumpIfTrue { target: l }
@@ -675,15 +669,17 @@ impl<C: Constant> CodeObject<C> {
     }
 
     pub fn map_bag<Bag: ConstantBag>(self, bag: &Bag) -> CodeObject<Bag::Constant> {
-        let map_names = |names: Vec<C::Name>| {
+        let map_names = |names: Box<[C::Name]>| {
             names
+                .into_vec()
                 .into_iter()
                 .map(|x| bag.make_name_ref(x.as_ref()))
-                .collect::<Vec<_>>()
+                .collect::<Box<[_]>>()
         };
         CodeObject {
             constants: self
                 .constants
+                .into_vec()
                 .into_iter()
                 .map(|x| x.map_constant(bag))
                 .collect(),
@@ -758,7 +754,7 @@ impl CodeObject<ConstantData> {
 impl<C: Constant> fmt::Display for CodeObject<C> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         self.display_inner(f, false, 1)?;
-        for constant in &self.constants {
+        for constant in &*self.constants {
             if let BorrowedConstant::Code { code } = constant.borrow_constant() {
                 write!(f, "\nDisassembly of {:?}\n", code)?;
                 code.fmt(f)?;
@@ -769,6 +765,7 @@ impl<C: Constant> fmt::Display for CodeObject<C> {
 }
 
 impl Instruction {
+    #[allow(clippy::too_many_arguments)]
     fn fmt_dis<C: Constant>(
         &self,
         f: &mut fmt::Formatter,
diff --git a/compiler/src/compile.rs b/compiler/src/compile.rs
index 051cb2cd5..32441d5d9 100644
--- a/compiler/src/compile.rs
+++ b/compiler/src/compile.rs
@@ -18,6 +18,9 @@ type CompileResult<T> = Result<T, CompileError>;
 
 struct CodeInfo {
     code: CodeObject,
+    instructions: Vec<Instruction>,
+    locations: Vec<bytecode::Location>,
+    constants: Vec<bytecode::ConstantData>,
     name_cache: IndexSet<String>,
     varname_cache: IndexSet<String>,
     cellvar_cache: IndexSet<String>,
@@ -28,6 +31,9 @@ impl CodeInfo {
     fn finalize_code(self) -> CodeObject {
         let CodeInfo {
             mut code,
+            instructions,
+            locations,
+            constants,
             name_cache,
             varname_cache,
             cellvar_cache,
@@ -35,10 +41,13 @@ impl CodeInfo {
             label_map,
         } = self;
 
-        code.names.extend(name_cache);
-        code.varnames.extend(varname_cache);
-        code.cellvars.extend(cellvar_cache);
-        code.freevars.extend(freevar_cache);
+        code.instructions = instructions.into();
+        code.locations = locations.into();
+        code.constants = constants.into();
+        code.names = name_cache.into_iter().collect();
+        code.varnames = varname_cache.into_iter().collect();
+        code.cellvars = cellvar_cache.into_iter().collect();
+        code.freevars = freevar_cache.into_iter().collect();
 
         if !code.cellvars.is_empty() {
             let total_args = code.arg_count
@@ -65,7 +74,7 @@ impl CodeInfo {
             }
         }
 
-        for instruction in &mut code.instructions {
+        for instruction in &mut *code.instructions {
             use Instruction::*;
             // this is a little bit hacky, as until now the data stored inside Labels in
             // Instructions is just bookkeeping, but I think it's the best way to do this
@@ -221,7 +230,7 @@ impl Compiler {
     fn new(opts: CompileOpts, source_path: String, code_name: String) -> Self {
         let module_code = CodeInfo {
             code: CodeObject::new(
-                Default::default(),
+                bytecode::CodeFlags::NEW_LOCALS,
                 0,
                 0,
                 0,
@@ -229,6 +238,9 @@ impl Compiler {
                 0,
                 code_name,
             ),
+            instructions: Vec::new(),
+            locations: Vec::new(),
+            constants: Vec::new(),
             name_cache: IndexSet::new(),
             varname_cache: IndexSet::new(),
             cellvar_cache: IndexSet::new(),
@@ -287,6 +299,9 @@ impl Compiler {
 
         let info = CodeInfo {
             code,
+            instructions: Vec::new(),
+            locations: Vec::new(),
+            constants: Vec::new(),
             name_cache: IndexSet::new(),
             varname_cache: IndexSet::new(),
             cellvar_cache,
@@ -315,7 +330,7 @@ impl Compiler {
         name: &str,
         cache: impl FnOnce(&mut CodeInfo) -> &mut IndexSet<String>,
     ) -> bytecode::NameIdx {
-        let cache = cache(self.code_stack.last_mut().expect("nothing on stack"));
+        let cache = cache(self.current_codeinfo());
         cache
             .get_index_of(name)
             .unwrap_or_else(|| cache.insert_full(name.to_owned()).0)
@@ -1160,7 +1175,7 @@ impl Compiler {
 
     fn build_closure(&mut self, code: &CodeObject) {
         if !code.freevars.is_empty() {
-            for var in &code.freevars {
+            for var in &*code.freevars {
                 let table = self.symbol_table_stack.last().unwrap();
                 let symbol = table.lookup(var).unwrap();
                 let parent_code = self.code_stack.last().unwrap();
@@ -2423,29 +2438,29 @@ impl Compiler {
     fn emit(&mut self, instruction: Instruction) {
         let location = compile_location(&self.current_source_location);
         // TODO: insert source filename
-        let code = self.current_code();
-        code.instructions.push(instruction);
-        code.locations.push(location);
+        let info = self.current_codeinfo();
+        info.instructions.push(instruction);
+        info.locations.push(location);
     }
 
     fn emit_constant(&mut self, constant: bytecode::ConstantData) {
-        let code = self.current_code();
-        let idx = code.constants.len();
-        code.constants.push(constant);
+        let info = self.current_codeinfo();
+        let idx = info.constants.len();
+        info.constants.push(constant);
         self.emit(Instruction::LoadConst { idx })
     }
 
     fn current_code(&mut self) -> &mut CodeObject {
-        &mut self
-            .code_stack
-            .last_mut()
-            .expect("No OutputStream on stack")
-            .code
+        &mut self.current_codeinfo().code
+    }
+
+    fn current_codeinfo(&mut self) -> &mut CodeInfo {
+        self.code_stack.last_mut().expect("no code on stack")
     }
 
     // Generate a new label
     fn new_label(&mut self) -> Label {
-        let label_map = &mut self.code_stack.last_mut().unwrap().label_map;
+        let label_map = &mut self.current_codeinfo().label_map;
         let label = Label(label_map.len());
         label_map.push(None);
         label
@@ -2454,9 +2469,11 @@ impl Compiler {
     // Assign current position the given label
     fn set_label(&mut self, label: Label) {
         let CodeInfo {
-            code, label_map, ..
-        } = self.code_stack.last_mut().unwrap();
-        let actual_label = Label(code.instructions.len());
+            instructions,
+            label_map,
+            ..
+        } = self.current_codeinfo();
+        let actual_label = Label(instructions.len());
         let prev_val = std::mem::replace(&mut label_map[label.0], Some(actual_label));
         debug_assert!(
             prev_val.map_or(true, |x| x == actual_label),
diff --git a/vm/src/builtins/function.rs b/vm/src/builtins/function.rs
index 125d54317..8aa8cdcdc 100644
--- a/vm/src/builtins/function.rs
+++ b/vm/src/builtins/function.rs
@@ -204,7 +204,7 @@ impl PyFunction {
             // Check if kw only arguments are all present:
             for (slot, kwarg) in fastlocals
                 .iter_mut()
-                .zip(&code.varnames)
+                .zip(&*code.varnames)
                 .skip(code.arg_count)
                 .take(code.kwonlyarg_count)
             {