Separate Overlay into its own thing (#344)

It's no longer attached to a Block. Makes access to overlays more
streamlined by removing this one indirection. Also makes it easier to
create standalone overlays without a block which might come in handy.

crates/nu-command/src/core_commands/use_.rs

@@ -40,8 +40,8 @@ impl Command for Use {
             ));
         };
 
-        if let Some(block_id) = engine_state.find_module(&import_pattern.head.name) {
-            let overlay = &engine_state.get_block(block_id).overlay;
+        if let Some(overlay_id) = engine_state.find_overlay(&import_pattern.head.name) {
+            let overlay = engine_state.get_overlay(overlay_id);
 
             let env_vars_to_use = if import_pattern.members.is_empty() {
                 overlay.env_vars_with_head(&import_pattern.head.name)

crates/nu-engine/src/eval.rs

@@ -525,7 +525,7 @@ pub fn eval_variable(
         let mut var_types = vec![];
         let mut commands = vec![];
         let mut aliases = vec![];
-        let mut modules = vec![];
+        let mut overlays = vec![];
 
         for frame in &engine_state.scope {
             for var in &frame.vars {
@@ -611,9 +611,9 @@ pub fn eval_variable(
                 });
             }
 
-            for module in &frame.modules {
-                modules.push(Value::String {
-                    val: String::from_utf8_lossy(module.0).to_string(),
+            for overlay in &frame.overlays {
+                overlays.push(Value::String {
+                    val: String::from_utf8_lossy(overlay.0).to_string(),
                     span,
                 });
             }
@@ -638,9 +638,9 @@ pub fn eval_variable(
             span,
         });
 
-        output_cols.push("modules".to_string());
+        output_cols.push("overlays".to_string());
         output_vals.push(Value::List {
-            vals: modules,
+            vals: overlays,
             span,
         });
 

crates/nu-parser/src/parse_keywords.rs

@@ -481,7 +481,7 @@ pub fn parse_export(
 pub fn parse_module_block(
     working_set: &mut StateWorkingSet,
     span: Span,
-) -> (Block, Option<ParseError>) {
+) -> (Block, Overlay, Option<ParseError>) {
     let mut error = None;
 
     working_set.enter_scope();
@@ -508,8 +508,6 @@ pub fn parse_module_block(
         .iter()
         .map(|pipeline| {
             if pipeline.commands.len() == 1 {
-                // this one here is doing parse_statement() equivalent
-                // let (stmt, err) = parse_statement(working_set, &pipeline.commands[0].parts);
                 let name = working_set.get_span_contents(pipeline.commands[0].parts[0]);
 
                 let (stmt, err) = match name {
@@ -571,7 +569,7 @@ pub fn parse_module_block(
 
     working_set.exit_scope();
 
-    (block.with_overlay(overlay), error)
+    (block, overlay, error)
 }
 
 pub fn parse_module(
@@ -584,7 +582,6 @@ pub fn parse_module(
     let mut error = None;
     let bytes = working_set.get_span_contents(spans[0]);
 
-    // parse_def() equivalent
     if bytes == b"module" && spans.len() >= 3 {
         let (module_name_expr, err) = parse_string(working_set, spans[1]);
         error = error.or(err);
@@ -593,7 +590,6 @@ pub fn parse_module(
             .as_string()
             .expect("internal error: module name is not a string");
 
-        // parse_block_expression() equivalent
         let block_span = spans[2];
         let block_bytes = working_set.get_span_contents(block_span);
         let mut start = block_span.start;
@@ -617,10 +613,11 @@ pub fn parse_module(
 
         let block_span = Span { start, end };
 
-        let (block, err) = parse_module_block(working_set, block_span);
+        let (block, overlay, err) = parse_module_block(working_set, block_span);
         error = error.or(err);
 
-        let block_id = working_set.add_module(&module_name, block);
+        let block_id = working_set.add_block(block);
+        let _ = working_set.add_overlay(&module_name, overlay);
 
         let block_expr = Expression {
             expr: Expr::Block(block_id),
@@ -679,11 +676,8 @@ pub fn parse_use(
         error = error.or(err);
 
         let (import_pattern, overlay) =
-            if let Some(block_id) = working_set.find_module(&import_pattern.head.name) {
-                (
-                    import_pattern,
-                    working_set.get_block(block_id).overlay.clone(),
-                )
+            if let Some(overlay_id) = working_set.find_overlay(&import_pattern.head.name) {
+                (import_pattern, working_set.get_overlay(overlay_id).clone())
             } else {
                 // TODO: Do not close over when loading module from file
                 // It could be a file
@@ -703,11 +697,12 @@ pub fn parse_use(
                         working_set.add_file(module_filename, &contents);
                         let span_end = working_set.next_span_start();
 
-                        let (block, err) =
+                        let (block, overlay, err) =
                             parse_module_block(working_set, Span::new(span_start, span_end));
                         error = error.or(err);
 
-                        let block_id = working_set.add_module(&module_name, block);
+                        let _ = working_set.add_block(block);
+                        let _ = working_set.add_overlay(&module_name, overlay.clone());
 
                         (
                             ImportPattern {
@@ -717,7 +712,7 @@ pub fn parse_use(
                                 },
                                 members: import_pattern.members,
                             },
-                            working_set.get_block(block_id).overlay.clone(),
+                            overlay,
                         )
                     } else {
                         return (
@@ -825,8 +820,8 @@ pub fn parse_hide(
         error = error.or(err);
 
         let (is_module, overlay) =
-            if let Some(block_id) = working_set.find_module(&import_pattern.head.name) {
-                (true, working_set.get_block(block_id).overlay.clone())
+            if let Some(overlay_id) = working_set.find_overlay(&import_pattern.head.name) {
+                (true, working_set.get_overlay(overlay_id).clone())
             } else if import_pattern.members.is_empty() {
                 // The pattern head can be e.g. a function name, not just a module
                 if let Some(id) = working_set.find_decl(&import_pattern.head.name) {

crates/nu-protocol/src/ast/block.rs

@@ -1,6 +1,6 @@
 use std::ops::{Index, IndexMut};
 
-use crate::{Overlay, Signature, VarId};
+use crate::{Signature, VarId};
 
 use super::Statement;
 
@@ -8,7 +8,6 @@ use super::Statement;
 pub struct Block {
     pub signature: Box<Signature>,
     pub stmts: Vec<Statement>,
-    pub overlay: Overlay,
     pub captures: Vec<VarId>,
 }
 
@@ -47,19 +46,9 @@ impl Block {
         Self {
             signature: Box::new(Signature::new("")),
             stmts: vec![],
-            overlay: Overlay::new(),
             captures: vec![],
         }
     }
-
-    pub fn with_overlay(self, overlay: Overlay) -> Self {
-        Self {
-            signature: self.signature,
-            stmts: self.stmts,
-            overlay,
-            captures: self.captures,
-        }
-    }
 }
 
 impl<T> From<T> for Block
@@ -70,7 +59,6 @@ where
         Self {
             signature: Box::new(Signature::new("")),
             stmts: stmts.collect(),
-            overlay: Overlay::new(),
             captures: vec![],
         }
     }

crates/nu-protocol/src/engine/engine_state.rs

@@ -1,5 +1,7 @@
 use super::Command;
-use crate::{ast::Block, BlockId, DeclId, Example, Signature, Span, Type, VarId};
+use crate::{
+    ast::Block, BlockId, DeclId, Example, Overlay, OverlayId, Signature, Span, Type, VarId,
+};
 use core::panic;
 use std::{
     collections::HashMap,
@@ -54,7 +56,7 @@ pub struct ScopeFrame {
     pub decls: HashMap<Vec<u8>, DeclId>,
     pub aliases: HashMap<Vec<u8>, Vec<Span>>,
     pub env_vars: HashMap<Vec<u8>, BlockId>,
-    pub modules: HashMap<Vec<u8>, BlockId>,
+    pub overlays: HashMap<Vec<u8>, OverlayId>,
     visibility: Visibility,
 }
 
@@ -66,7 +68,7 @@ impl ScopeFrame {
             decls: HashMap::new(),
             aliases: HashMap::new(),
             env_vars: HashMap::new(),
-            modules: HashMap::new(),
+            overlays: HashMap::new(),
             visibility: Visibility::new(),
         }
     }
@@ -131,6 +133,7 @@ pub struct EngineState {
     vars: im::Vector<Type>,
     decls: im::Vector<Box<dyn Command + 'static>>,
     blocks: im::Vector<Block>,
+    overlays: im::Vector<Overlay>,
     pub scope: im::Vector<ScopeFrame>,
     pub ctrlc: Option<Arc<AtomicBool>>,
 }
@@ -148,6 +151,7 @@ impl EngineState {
             vars: im::vector![Type::Unknown, Type::Unknown, Type::Unknown, Type::Unknown],
             decls: im::vector![],
             blocks: im::vector![],
+            overlays: im::vector![],
             scope: im::vector![ScopeFrame::new()],
             ctrlc: None,
         }
@@ -167,6 +171,7 @@ impl EngineState {
         self.decls.extend(delta.decls);
         self.vars.extend(delta.vars);
         self.blocks.extend(delta.blocks);
+        self.overlays.extend(delta.overlays);
 
         if let Some(last) = self.scope.back_mut() {
             let first = delta.scope.remove(0);
@@ -179,8 +184,8 @@ impl EngineState {
             for item in first.aliases.into_iter() {
                 last.aliases.insert(item.0, item.1);
             }
-            for item in first.modules.into_iter() {
-                last.modules.insert(item.0, item.1);
+            for item in first.overlays.into_iter() {
+                last.overlays.insert(item.0, item.1);
             }
             last.visibility.merge_with(first.visibility);
         }
@@ -202,6 +207,10 @@ impl EngineState {
         self.blocks.len()
     }
 
+    pub fn num_overlays(&self) -> usize {
+        self.overlays.len()
+    }
+
     pub fn print_vars(&self) {
         for var in self.vars.iter().enumerate() {
             println!("var{}: {:?}", var.0, var.1);
@@ -243,10 +252,10 @@ impl EngineState {
         None
     }
 
-    pub fn find_module(&self, name: &[u8]) -> Option<BlockId> {
+    pub fn find_overlay(&self, name: &[u8]) -> Option<OverlayId> {
         for scope in self.scope.iter().rev() {
-            if let Some(block_id) = scope.modules.get(name) {
-                return Some(*block_id);
+            if let Some(overlay_id) = scope.overlays.get(name) {
+                return Some(*overlay_id);
             }
         }
 
@@ -326,6 +335,12 @@ impl EngineState {
             .expect("internal error: missing block")
     }
 
+    pub fn get_overlay(&self, overlay_id: OverlayId) -> &Overlay {
+        self.overlays
+            .get(overlay_id)
+            .expect("internal error: missing overlay")
+    }
+
     pub fn next_span_start(&self) -> usize {
         if let Some((_, _, last)) = self.file_contents.last() {
             *last
@@ -404,6 +419,7 @@ pub struct StateDelta {
     vars: Vec<Type>,              // indexed by VarId
     decls: Vec<Box<dyn Command>>, // indexed by DeclId
     blocks: Vec<Block>,           // indexed by BlockId
+    overlays: Vec<Overlay>,       // indexed by OverlayId
     pub scope: Vec<ScopeFrame>,
 }
 
@@ -420,6 +436,10 @@ impl StateDelta {
         self.blocks.len()
     }
 
+    pub fn num_overlays(&self) -> usize {
+        self.overlays.len()
+    }
+
     pub fn enter_scope(&mut self) {
         self.scope.push(ScopeFrame::new());
     }
@@ -438,6 +458,7 @@ impl<'a> StateWorkingSet<'a> {
                 vars: vec![],
                 decls: vec![],
                 blocks: vec![],
+                overlays: vec![],
                 scope: vec![ScopeFrame::new()],
             },
             permanent_state,
@@ -456,6 +477,10 @@ impl<'a> StateWorkingSet<'a> {
         self.delta.num_blocks() + self.permanent_state.num_blocks()
     }
 
+    pub fn num_overlays(&self) -> usize {
+        self.delta.num_overlays() + self.permanent_state.num_overlays()
+    }
+
     pub fn add_decl(&mut self, decl: Box<dyn Command>) -> DeclId {
         let name = decl.name().as_bytes().to_vec();
 
@@ -581,11 +606,11 @@ impl<'a> StateWorkingSet<'a> {
         block_id
     }
 
-    pub fn add_module(&mut self, name: &str, block: Block) -> BlockId {
+    pub fn add_overlay(&mut self, name: &str, overlay: Overlay) -> OverlayId {
         let name = name.as_bytes().to_vec();
 
-        self.delta.blocks.push(block);
-        let block_id = self.num_blocks() - 1;
+        self.delta.overlays.push(overlay);
+        let overlay_id = self.num_overlays() - 1;
 
         let scope_frame = self
             .delta
@@ -593,9 +618,9 @@ impl<'a> StateWorkingSet<'a> {
             .last_mut()
             .expect("internal error: missing required scope frame");
 
-        scope_frame.modules.insert(name, block_id);
+        scope_frame.overlays.insert(name, overlay_id);
 
-        block_id
+        overlay_id
     }
 
     pub fn next_span_start(&self) -> usize {
@@ -708,16 +733,16 @@ impl<'a> StateWorkingSet<'a> {
         None
     }
 
-    pub fn find_module(&self, name: &[u8]) -> Option<BlockId> {
+    pub fn find_overlay(&self, name: &[u8]) -> Option<OverlayId> {
         for scope in self.delta.scope.iter().rev() {
-            if let Some(block_id) = scope.modules.get(name) {
-                return Some(*block_id);
+            if let Some(overlay_id) = scope.overlays.get(name) {
+                return Some(*overlay_id);
             }
         }
 
         for scope in self.permanent_state.scope.iter().rev() {
-            if let Some(block_id) = scope.modules.get(name) {
-                return Some(*block_id);
+            if let Some(overlay_id) = scope.overlays.get(name) {
+                return Some(*overlay_id);
             }
         }
 
@@ -893,6 +918,18 @@ impl<'a> StateWorkingSet<'a> {
         }
     }
 
+    pub fn get_overlay(&self, overlay_id: OverlayId) -> &Overlay {
+        let num_permanent_overlays = self.permanent_state.num_overlays();
+        if overlay_id < num_permanent_overlays {
+            self.permanent_state.get_overlay(overlay_id)
+        } else {
+            self.delta
+                .overlays
+                .get(overlay_id - num_permanent_overlays)
+                .expect("internal error: missing overlay")
+        }
+    }
+
     pub fn get_block_mut(&mut self, block_id: BlockId) -> &mut Block {
         let num_permanent_blocks = self.permanent_state.num_blocks();
         if block_id < num_permanent_blocks {

crates/nu-protocol/src/id.rs

@@ -1,3 +1,4 @@
 pub type VarId = usize;
 pub type DeclId = usize;
 pub type BlockId = usize;
+pub type OverlayId = usize;