Rewrite SyntaxEnv

I'd really like to be able to do something like

struct MapChain<'next, K, V> {
    info: BlockInfo,
    map: HashMap<K, V>,
    next: Option<&'next mut MapChain<'next, K, V>
}

but I can't get the lifetimes to work out.

Author: sfackler

src/libsyntax/ext/base.rs

@@ -156,57 +156,48 @@ pub enum SyntaxExtension {
 // The SyntaxEnv is the environment that's threaded through the expansion
 // of macros. It contains bindings for macros, and also a special binding
 // for " block" (not a legal identifier) that maps to a BlockInfo
-pub type SyntaxEnv = @mut MapChain<Name, Transformer>;
-
-// Transformer : the codomain of SyntaxEnvs
-
-pub enum Transformer {
-    // this identifier maps to a syntax extension or macro
-    SE(SyntaxExtension),
-    // blockinfo : this is ... well, it's simpler than threading
-    // another whole data stack-structured data structure through
-    // expansion. Basically, there's an invariant that every
-    // map must contain a binding for " block".
-    BlockInfo(BlockInfo)
-}
+pub type SyntaxEnv = MapChain<Name, SyntaxExtension>;
 
 pub struct BlockInfo {
     // should macros escape from this scope?
     macros_escape : bool,
     // what are the pending renames?
-    pending_renames : @mut RenameList
+    pending_renames : RenameList
+}
+
+impl BlockInfo {
+    pub fn new() -> BlockInfo {
+        BlockInfo {
+            macros_escape: false,
+            pending_renames: ~[]
+        }
+    }
 }
 
 // a list of ident->name renamings
-type RenameList = ~[(ast::Ident,Name)];
+pub type RenameList = ~[(ast::Ident,Name)];
 
 // The base map of methods for expanding syntax extension
 // AST nodes into full ASTs
 pub fn syntax_expander_table() -> SyntaxEnv {
     // utility function to simplify creating NormalTT syntax extensions
     fn builtin_normal_tt_no_ctxt(f: SyntaxExpanderTTFunNoCtxt)
-                                 -> @Transformer {
-        @SE(NormalTT(@SyntaxExpanderTT{
+                                 -> SyntaxExtension {
+        NormalTT(@SyntaxExpanderTT{
             expander: SyntaxExpanderTTExpanderWithoutContext(f),
             span: None,
         } as @SyntaxExpanderTTTrait,
-        None))
+        None)
     }
 
-    let mut syntax_expanders = HashMap::new();
-    // NB identifier starts with space, and can't conflict with legal idents
-    syntax_expanders.insert(intern(&" block"),
-                            @BlockInfo(BlockInfo{
-                                macros_escape : false,
-                                pending_renames : @mut ~[]
-                            }));
+    let mut syntax_expanders = MapChain::new();
     syntax_expanders.insert(intern(&"macro_rules"),
-                            @SE(IdentTT(@SyntaxExpanderTTItem {
+                            IdentTT(@SyntaxExpanderTTItem {
                                 expander: SyntaxExpanderTTItemExpanderWithContext(
                                     ext::tt::macro_rules::add_new_extension),
                                 span: None,
                             } as @SyntaxExpanderTTItemTrait,
-                            None)));
+                            None));
     syntax_expanders.insert(intern(&"fmt"),
                             builtin_normal_tt_no_ctxt(
                                 ext::fmt::expand_syntax_ext));
@@ -232,8 +223,7 @@ pub fn syntax_expander_table() -> SyntaxEnv {
                             builtin_normal_tt_no_ctxt(
                                     ext::log_syntax::expand_syntax_ext));
     syntax_expanders.insert(intern(&"deriving"),
-                            @SE(ItemDecorator(
-                                ext::deriving::expand_meta_deriving)));
+                            ItemDecorator(ext::deriving::expand_meta_deriving));
 
     // Quasi-quoting expanders
     syntax_expanders.insert(intern(&"quote_tokens"),
@@ -288,7 +278,7 @@ pub fn syntax_expander_table() -> SyntaxEnv {
     syntax_expanders.insert(intern(&"trace_macros"),
                             builtin_normal_tt_no_ctxt(
                                     ext::trace_macros::expand_trace_macros));
-    MapChain::new(~syntax_expanders)
+    syntax_expanders
 }
 
 // One of these is made during expansion and incrementally updated as we go;
@@ -299,11 +289,6 @@ pub struct ExtCtxt {
     cfg: ast::CrateConfig,
     backtrace: Option<@ExpnInfo>,
 
-    // These two @mut's should really not be here,
-    // but the self types for CtxtRepr are all wrong
-    // and there are bugs in the code for object
-    // types that make this hard to get right at the
-    // moment. - nmatsakis
     mod_path: ~[ast::Ident],
     trace_mac: bool
 }
@@ -325,7 +310,7 @@ impl ExtCtxt {
             match e.node {
                 ast::ExprMac(..) => {
                     let mut expander = expand::MacroExpander {
-                        extsbox: @mut syntax_expander_table(),
+                        extsbox: syntax_expander_table(),
                         cx: self,
                     };
                     e = expand::expand_expr(e, &mut expander);
@@ -460,11 +445,7 @@ pub fn get_exprs_from_tts(cx: &ExtCtxt,
 // we want to implement the notion of a transformation
 // environment.
 
-// This environment maps Names to Transformers.
-// Initially, this includes macro definitions and
-// block directives.
-
-
+// This environment maps Names to SyntaxExtensions.
 
 // Actually, the following implementation is parameterized
 // by both key and value types.
@@ -479,169 +460,98 @@ pub fn get_exprs_from_tts(cx: &ExtCtxt,
 // able to refer to a macro that was added to an enclosing
 // scope lexically later than the deeper scope.
 
-// Note on choice of representation: I've been pushed to
-// use a top-level managed pointer by some difficulties
-// with pushing and popping functionally, and the ownership
-// issues.  As a result, the values returned by the table
-// also need to be managed; the &'a ... type that Maps
-// return won't work for things that need to get outside
-// of that managed pointer.  The easiest way to do this
-// is just to insist that the values in the tables are
-// managed to begin with.
-
-// a transformer env is either a base map or a map on top
-// of another chain.
-pub enum MapChain<K,V> {
-    BaseMapChain(~HashMap<K,@V>),
-    ConsMapChain(~HashMap<K,@V>,@mut MapChain<K,V>)
+// Only generic to make it easy to test
+struct MapChainFrame<K, V> {
+    info: BlockInfo,
+    map: HashMap<K, V>,
 }
 
+// Only generic to make it easy to test
+pub struct MapChain<K, V> {
+    priv chain: ~[MapChainFrame<K, V>],
+}
 
-// get the map from an env frame
-impl <K: Eq + Hash + IterBytes + 'static, V: 'static> MapChain<K,V>{
-    // Constructor. I don't think we need a zero-arg one.
-    pub fn new(init: ~HashMap<K,@V>) -> @mut MapChain<K,V> {
-        @mut BaseMapChain(init)
+impl<K: Hash+Eq, V> MapChain<K, V> {
+    pub fn new() -> MapChain<K, V> {
+        let mut map = MapChain { chain: ~[] };
+        map.push_frame();
+        map
     }
 
-    // add a new frame to the environment (functionally)
-    pub fn push_frame (@mut self) -> @mut MapChain<K,V> {
-        @mut ConsMapChain(~HashMap::new() ,self)
+    pub fn push_frame(&mut self) {
+        self.chain.push(MapChainFrame {
+            info: BlockInfo::new(),
+            map: HashMap::new(),
+        });
     }
 
-// no need for pop, it'll just be functional.
-
-    // utility fn...
-
-    // ugh: can't get this to compile with mut because of the
-    // lack of flow sensitivity.
-    pub fn get_map<'a>(&'a self) -> &'a HashMap<K,@V> {
-        match *self {
-            BaseMapChain (~ref map) => map,
-            ConsMapChain (~ref map,_) => map
-        }
+    pub fn pop_frame(&mut self) {
+        assert!(self.chain.len() > 1, "too many pops on MapChain!");
+        self.chain.pop();
     }
 
-// traits just don't work anywhere...?
-//impl Map<Name,SyntaxExtension> for MapChain {
-
-    pub fn contains_key (&self, key: &K) -> bool {
-        match *self {
-            BaseMapChain (ref map) => map.contains_key(key),
-            ConsMapChain (ref map,ref rest) =>
-            (map.contains_key(key)
-             || rest.contains_key(key))
+    fn find_escape_frame<'a>(&'a mut self) -> &'a mut MapChainFrame<K, V> {
+        for (i, frame) in self.chain.mut_iter().enumerate().invert() {
+            if !frame.info.macros_escape || i == 0 {
+                return frame
+            }
         }
-    }
-    // should each_key and each_value operate on shadowed
-    // names? I think not.
-    // delaying implementing this....
-    pub fn each_key (&self, _f: |&K| -> bool) {
-        fail!("unimplemented 2013-02-15T10:01");
-    }
-
-    pub fn each_value (&self, _f: |&V| -> bool) {
-        fail!("unimplemented 2013-02-15T10:02");
+        unreachable!()
     }
 
-    // Returns a copy of the value that the name maps to.
-    // Goes down the chain 'til it finds one (or bottom out).
-    pub fn find (&self, key: &K) -> Option<@V> {
-        match self.get_map().find (key) {
-            Some(ref v) => Some(**v),
-            None => match *self {
-                BaseMapChain (_) => None,
-                ConsMapChain (_,ref rest) => rest.find(key)
+    pub fn find<'a>(&'a self, k: &K) -> Option<&'a V> {
+        for frame in self.chain.iter().invert() {
+            match frame.map.find(k) {
+                Some(v) => return Some(v),
+                None => {}
             }
         }
+        None
     }
 
-    pub fn find_in_topmost_frame(&self, key: &K) -> Option<@V> {
-        let map = match *self {
-            BaseMapChain(ref map) => map,
-            ConsMapChain(ref map,_) => map
-        };
-        // strip one layer of indirection off the pointer.
-        map.find(key).map(|r| {*r})
-    }
-
-    // insert the binding into the top-level map
-    pub fn insert (&mut self, key: K, ext: @V) -> bool {
-        // can't abstract over get_map because of flow sensitivity...
-        match *self {
-            BaseMapChain (~ref mut map) => map.insert(key, ext),
-            ConsMapChain (~ref mut map,_) => map.insert(key,ext)
-        }
+    pub fn insert(&mut self, k: K, v: V) {
+        self.find_escape_frame().map.insert(k, v);
     }
-    // insert the binding into the topmost frame for which the binding
-    // associated with 'n' exists and satisfies pred
-    // ... there are definitely some opportunities for abstraction
-    // here that I'm ignoring. (e.g., manufacturing a predicate on
-    // the maps in the chain, and using an abstract "find".
-    pub fn insert_into_frame(&mut self,
-                             key: K,
-                             ext: @V,
-                             n: K,
-                             pred: |&@V| -> bool) {
-        match *self {
-            BaseMapChain (~ref mut map) => {
-                if satisfies_pred(map,&n,pred) {
-                    map.insert(key,ext);
-                } else {
-                    fail!("expected map chain containing satisfying frame")
-                }
-            },
-            ConsMapChain (~ref mut map, rest) => {
-                if satisfies_pred(map,&n,|v|pred(v)) {
-                    map.insert(key,ext);
-                } else {
-                    rest.insert_into_frame(key,ext,n,pred)
-                }
-            }
-        }
-    }
-}
 
-// returns true if the binding for 'n' satisfies 'pred' in 'map'
-fn satisfies_pred<K:Eq + Hash + IterBytes,
-                  V>(
-                  map: &mut HashMap<K,V>,
-                  n: &K,
-                  pred: |&V| -> bool)
-                  -> bool {
-    match map.find(n) {
-        Some(ref v) => (pred(*v)),
-        None => false
+    pub fn info<'a>(&'a mut self) -> &'a mut BlockInfo {
+        &mut self.chain[self.chain.len()-1].info
     }
 }
 
 #[cfg(test)]
 mod test {
     use super::MapChain;
-    use std::hashmap::HashMap;
 
     #[test]
     fn testenv() {
-        let mut a = HashMap::new();
-        a.insert (@"abc",@15);
-        let m = MapChain::new(~a);
-        m.insert (@"def",@16);
-        assert_eq!(m.find(&@"abc"),Some(@15));
-        assert_eq!(m.find(&@"def"),Some(@16));
-        assert_eq!(*(m.find(&@"abc").unwrap()),15);
-        assert_eq!(*(m.find(&@"def").unwrap()),16);
-        let n = m.push_frame();
-        // old bindings are still present:
-        assert_eq!(*(n.find(&@"abc").unwrap()),15);
-        assert_eq!(*(n.find(&@"def").unwrap()),16);
-        n.insert (@"def",@17);
-        // n shows the new binding
-        assert_eq!(*(n.find(&@"abc").unwrap()),15);
-        assert_eq!(*(n.find(&@"def").unwrap()),17);
-        // ... but m still has the old ones
-        assert_eq!(m.find(&@"abc"),Some(@15));
-        assert_eq!(m.find(&@"def"),Some(@16));
-        assert_eq!(*(m.find(&@"abc").unwrap()),15);
-        assert_eq!(*(m.find(&@"def").unwrap()),16);
+        let mut m = MapChain::new();
+        let (a,b,c,d) = ("a", "b", "c", "d");
+        m.insert(1, a);
+        assert_eq!(Some(&a), m.find(&1));
+
+        m.push_frame();
+        m.info().macros_escape = true;
+        m.insert(2, b);
+        assert_eq!(Some(&a), m.find(&1));
+        assert_eq!(Some(&b), m.find(&2));
+        m.pop_frame();
+
+        assert_eq!(Some(&a), m.find(&1));
+        assert_eq!(Some(&b), m.find(&2));
+
+        m.push_frame();
+        m.push_frame();
+        m.info().macros_escape = true;
+        m.insert(3, c);
+        assert_eq!(Some(&c), m.find(&3));
+        m.pop_frame();
+        assert_eq!(Some(&c), m.find(&3));
+        m.pop_frame();
+        assert_eq!(None, m.find(&3));
+
+        m.push_frame();
+        m.insert(4, d);
+        m.pop_frame();
+        assert_eq!(None, m.find(&4));
     }
 }