auto merge of #13261 : pnkfelix/rust/fsk-fix-12856, r=nikomatsakis

Fix #12856.

I wanted to put this up first because I wanted to get feedback about the second commit in the series, commit 8599236.  Its the more invasive part of the patch and is largely just belt-and-suspenders assertion checking; in the commit message I mentioned at least one other approach we could take here.  Or we could drop the belt-and-suspenders and just rely on the guard added in the first patch, commit 8d6a005 (which is really quite trivial on its own).

So any feedback on what would be better is appreciated.

r? @nikomatsakis

Author: bors

src/librustc/middle/trans/monomorphize.rs

@@ -272,6 +272,7 @@ pub fn monomorphic_fn(ccx: &CrateContext,
         }
 
         // Ugh -- but this ensures any new variants won't be forgotten
+        ast_map::NodeLifetime(..) |
         ast_map::NodeExpr(..) |
         ast_map::NodeStmt(..) |
         ast_map::NodeArg(..) |

src/librustc/middle/typeck/variance.rs

@@ -198,6 +198,7 @@ use arena::Arena;
 use middle::ty;
 use std::fmt;
 use syntax::ast;
+use syntax::ast_map;
 use syntax::ast_util;
 use syntax::owned_slice::OwnedSlice;
 use syntax::visit;
@@ -517,6 +518,13 @@ impl<'a> Visitor<()> for ConstraintContext<'a> {
     }
 }
 
+/// Is `param_id` a lifetime according to `map`?
+fn is_lifetime(map: &ast_map::Map, param_id: ast::NodeId) -> bool {
+    match map.find(param_id) {
+        Some(ast_map::NodeLifetime(..)) => true, _ => false
+    }
+}
+
 impl<'a> ConstraintContext<'a> {
     fn tcx(&self) -> &'a ty::ctxt {
         self.terms_cx.tcx
@@ -533,6 +541,70 @@ impl<'a> ConstraintContext<'a> {
         }
     }
 
+    fn find_binding_for_lifetime(&self, param_id: ast::NodeId) -> ast::NodeId {
+        let tcx = self.terms_cx.tcx;
+        assert!(is_lifetime(&tcx.map, param_id));
+        match tcx.named_region_map.find(&param_id) {
+            Some(&ast::DefEarlyBoundRegion(_, lifetime_decl_id))
+                => lifetime_decl_id,
+            Some(_) => fail!("should not encounter non early-bound cases"),
+
+            // The lookup should only fail when `param_id` is
+            // itself a lifetime binding: use it as the decl_id.
+            None    => param_id,
+        }
+
+    }
+
+    /// Is `param_id` a type parameter for which we infer variance?
+    fn is_to_be_inferred(&self, param_id: ast::NodeId) -> bool {
+        let result = self.terms_cx.inferred_map.contains_key(&param_id);
+
+        // To safe-guard against invalid inferred_map constructions,
+        // double-check if variance is inferred at some use of a type
+        // parameter (by inspecting parent of its binding declaration
+        // to see if it is introduced by a type or by a fn/impl).
+
+        let check_result = |this:&ConstraintContext| -> bool {
+            let tcx = this.terms_cx.tcx;
+            let decl_id = this.find_binding_for_lifetime(param_id);
+            // Currently only called on lifetimes; double-checking that.
+            assert!(is_lifetime(&tcx.map, param_id));
+            let parent_id = tcx.map.get_parent(decl_id);
+            let parent = tcx.map.find(parent_id).unwrap_or_else(
+                || fail!("tcx.map missing entry for id: {}", parent_id));
+
+            let is_inferred;
+            macro_rules! cannot_happen { () => { {
+                fail!("invalid parent: {:s} for {:s}",
+                      tcx.map.node_to_str(parent_id),
+                      tcx.map.node_to_str(param_id));
+            } } }
+
+            match parent {
+                ast_map::NodeItem(p) => {
+                    match p.node {
+                        ast::ItemTy(..) |
+                        ast::ItemEnum(..) |
+                        ast::ItemStruct(..) |
+                        ast::ItemTrait(..)   => is_inferred = true,
+                        ast::ItemFn(..)      => is_inferred = false,
+                        _                    => cannot_happen!(),
+                    }
+                }
+                ast_map::NodeTraitMethod(..) => is_inferred = false,
+                ast_map::NodeMethod(_)       => is_inferred = false,
+                _                            => cannot_happen!(),
+            }
+
+            return is_inferred;
+        };
+
+        assert_eq!(result, check_result(self));
+
+        return result;
+    }
+
     fn declared_variance(&self,
                          param_def_id: ast::DefId,
                          item_def_id: ast::DefId,
@@ -788,8 +860,10 @@ impl<'a> ConstraintContext<'a> {
                                    variance: VarianceTermPtr<'a>) {
         match region {
             ty::ReEarlyBound(param_id, _, _) => {
-                let index = self.inferred_index(param_id);
-                self.add_constraint(index, variance);
+                if self.is_to_be_inferred(param_id) {
+                    let index = self.inferred_index(param_id);
+                    self.add_constraint(index, variance);
+                }
             }
 
             ty::ReStatic => { }

src/libsyntax/ast_map.rs

@@ -107,6 +107,8 @@ pub enum Node {
 
     /// NodeStructCtor represents a tuple struct.
     NodeStructCtor(@StructDef),
+
+    NodeLifetime(@Lifetime),
 }
 
 // The odd layout is to bring down the total size.
@@ -127,6 +129,7 @@ enum MapEntry {
     EntryLocal(NodeId, @Pat),
     EntryBlock(NodeId, P<Block>),
     EntryStructCtor(NodeId, @StructDef),
+    EntryLifetime(NodeId, @Lifetime),
 
     // Roots for node trees.
     RootCrate,
@@ -153,6 +156,7 @@ impl MapEntry {
             EntryLocal(id, _) => id,
             EntryBlock(id, _) => id,
             EntryStructCtor(id, _) => id,
+            EntryLifetime(id, _) => id,
             _ => return None
         })
     }
@@ -170,6 +174,7 @@ impl MapEntry {
             EntryLocal(_, p) => NodeLocal(p),
             EntryBlock(_, p) => NodeBlock(p),
             EntryStructCtor(_, p) => NodeStructCtor(p),
+            EntryLifetime(_, p) => NodeLifetime(p),
             _ => return None
         })
     }
@@ -213,6 +218,8 @@ impl Map {
         self.find_entry(id).and_then(|x| x.to_node())
     }
 
+    /// Retrieve the parent NodeId for `id`, or `id` itself if no
+    /// parent is registered in this map.
     pub fn get_parent(&self, id: NodeId) -> NodeId {
         self.find_entry(id).and_then(|x| x.parent()).unwrap_or(id)
     }
@@ -500,6 +507,15 @@ impl<'a, F: FoldOps> Folder for Ctx<'a, F> {
         SmallVector::one(stmt)
     }
 
+    fn fold_type_method(&mut self, m: &TypeMethod) -> TypeMethod {
+        let parent = self.parent;
+        self.parent = DUMMY_NODE_ID;
+        let m = fold::noop_fold_type_method(m, self);
+        assert_eq!(self.parent, m.id);
+        self.parent = parent;
+        m
+    }
+
     fn fold_method(&mut self, m: @Method) -> @Method {
         let parent = self.parent;
         self.parent = DUMMY_NODE_ID;
@@ -522,6 +538,12 @@ impl<'a, F: FoldOps> Folder for Ctx<'a, F> {
         self.insert(block.id, EntryBlock(self.parent, block));
         block
     }
+
+    fn fold_lifetime(&mut self, lifetime: &Lifetime) -> Lifetime {
+        let lifetime = fold::noop_fold_lifetime(lifetime, self);
+        self.insert(lifetime.id, EntryLifetime(self.parent, @lifetime));
+        lifetime
+    }
 }
 
 pub fn map_crate<F: FoldOps>(krate: Crate, fold_ops: F) -> (Crate, Map) {
@@ -658,6 +680,9 @@ fn node_id_to_str(map: &Map, id: NodeId) -> ~str {
         Some(NodeStructCtor(_)) => {
             format!("struct_ctor {} (id={})", map.path_to_str(id), id)
         }
+        Some(NodeLifetime(ref l)) => {
+            format!("lifetime {} (id={})", pprust::lifetime_to_str(*l), id)
+        }
         None => {
             format!("unknown node (id={})", id)
         }