Auto merge of #26370 - nikomatsakis:better-object-defaults-warn, r=nikomatsakis

This is an implementation of RFC rust-lang/rfcs#1156. It includes the code to implement the new rules, but that code is currently disabled. It also includes code to issue warnings when the change will cause breakage. These warnings try hard to be targeted but are also somewhat approximate. They could, with some effort, be made *more* targeted by adjusting the code in ty_relate that propagates the "will change" flag to consider the specific operation. Might be worth doing.

r? @pnkfelix (I think you understand region inference best)

Author: bors

src/librustc/diagnostics.rs

@@ -1160,6 +1160,42 @@ static mut FOO: Option<Box<usize>> = None;
 // error: mutable statics are not allowed to have destructors
 static mut BAR: Option<Vec<i32>> = None;
 ```
+"##,
+
+E0398: r##"
+In Rust 1.3, the default object lifetime bounds are expected to
+change, as described in RFC #1156 [1]. You are getting a warning
+because the compiler thinks it is possible that this change will cause
+a compilation error in your code. It is possible, though unlikely,
+that this is a false alarm.
+
+The heart of the change is that where `&'a Box<SomeTrait>` used to
+default to `&'a Box<SomeTrait+'a>`, it now defaults to `&'a
+Box<SomeTrait+'static>` (here, `SomeTrait` is the name of some trait
+type). Note that the only types which are affected are references to
+boxes, like `&Box<SomeTrait>` or `&[Box<SomeTrait>]`.  More common
+types like `&SomeTrait` or `Box<SomeTrait>` are unaffected.
+
+To silence this warning, edit your code to use an explicit bound.
+Most of the time, this means that you will want to change the
+signature of a function that you are calling. For example, if
+the error is reported on a call like `foo(x)`, and `foo` is
+defined as follows:
+
+```
+fn foo(arg: &Box<SomeTrait>) { ... }
+```
+
+you might change it to:
+
+```
+fn foo<'a>(arg: &Box<SomeTrait+'a>) { ... }
+```
+
+This explicitly states that you expect the trait object `SomeTrait` to
+contain references (with a maximum lifetime of `'a`).
+
+[1]: https://github.com/rust-lang/rfcs/pull/1156
 "##
 
 }

src/librustc/metadata/tydecode.rs

@@ -843,15 +843,15 @@ fn parse_type_param_def_<'a, 'tcx, F>(st: &mut PState<'a, 'tcx>, conv: &mut F)
 
 fn parse_object_lifetime_default<'a,'tcx, F>(st: &mut PState<'a,'tcx>,
                                              conv: &mut F)
-                                             -> Option<ty::ObjectLifetimeDefault>
+                                             -> ty::ObjectLifetimeDefault
     where F: FnMut(DefIdSource, ast::DefId) -> ast::DefId,
 {
     match next(st) {
-        'n' => None,
-        'a' => Some(ty::ObjectLifetimeDefault::Ambiguous),
+        'a' => ty::ObjectLifetimeDefault::Ambiguous,
+        'b' => ty::ObjectLifetimeDefault::BaseDefault,
         's' => {
             let region = parse_region_(st, conv);
-            Some(ty::ObjectLifetimeDefault::Specific(region))
+            ty::ObjectLifetimeDefault::Specific(region)
         }
         _ => panic!("parse_object_lifetime_default: bad input")
     }
@@ -887,9 +887,16 @@ fn parse_existential_bounds_<'a,'tcx, F>(st: &mut PState<'a,'tcx>,
         }
     }
 
+    let region_bound_will_change = match next(st) {
+        'y' => true,
+        'n' => false,
+        c => panic!("parse_ty: expected y/n not '{}'", c)
+    };
+
     return ty::ExistentialBounds { region_bound: region_bound,
                                    builtin_bounds: builtin_bounds,
-                                   projection_bounds: projection_bounds };
+                                   projection_bounds: projection_bounds,
+                                   region_bound_will_change: region_bound_will_change };
 }
 
 fn parse_builtin_bounds<F>(st: &mut PState, mut _conv: F) -> ty::BuiltinBounds where

src/librustc/metadata/tyencode.rs

@@ -390,6 +390,8 @@ pub fn enc_existential_bounds<'a,'tcx>(w: &mut Encoder,
     }
 
     mywrite!(w, ".");
+
+    mywrite!(w, "{}", if bs.region_bound_will_change {'y'} else {'n'});
 }
 
 pub fn enc_region_bounds<'a, 'tcx>(w: &mut Encoder,
@@ -414,12 +416,12 @@ pub fn enc_type_param_def<'a, 'tcx>(w: &mut Encoder, cx: &ctxt<'a, 'tcx>,
 
 fn enc_object_lifetime_default<'a, 'tcx>(w: &mut Encoder,
                                          cx: &ctxt<'a, 'tcx>,
-                                         default: Option<ty::ObjectLifetimeDefault>)
+                                         default: ty::ObjectLifetimeDefault)
 {
     match default {
-        None => mywrite!(w, "n"),
-        Some(ty::ObjectLifetimeDefault::Ambiguous) => mywrite!(w, "a"),
-        Some(ty::ObjectLifetimeDefault::Specific(r)) => {
+        ty::ObjectLifetimeDefault::Ambiguous => mywrite!(w, "a"),
+        ty::ObjectLifetimeDefault::BaseDefault => mywrite!(w, "b"),
+        ty::ObjectLifetimeDefault::Specific(r) => {
             mywrite!(w, "s");
             enc_region(w, cx, r);
         }

src/librustc/middle/infer/bivariate.rs

@@ -49,6 +49,11 @@ impl<'a, 'tcx> TypeRelation<'a, 'tcx> for Bivariate<'a, 'tcx> {
 
     fn a_is_expected(&self) -> bool { self.fields.a_is_expected }
 
+    fn will_change(&mut self, _: bool, _: bool) -> bool {
+        // since we are not comparing regions, we don't care
+        false
+    }
+
     fn relate_with_variance<T:Relate<'a,'tcx>>(&mut self,
                                                variance: ty::Variance,
                                                a: &T,

src/librustc/middle/infer/combine.rs

@@ -56,6 +56,7 @@ pub struct CombineFields<'a, 'tcx: 'a> {
     pub infcx: &'a InferCtxt<'a, 'tcx>,
     pub a_is_expected: bool,
     pub trace: TypeTrace<'tcx>,
+    pub cause: Option<ty_relate::Cause>,
 }
 
 pub fn super_combine_tys<'a,'tcx:'a,R>(infcx: &InferCtxt<'a, 'tcx>,

src/librustc/middle/infer/equate.rs

@@ -34,6 +34,11 @@ impl<'a, 'tcx> TypeRelation<'a,'tcx> for Equate<'a, 'tcx> {
 
     fn a_is_expected(&self) -> bool { self.fields.a_is_expected }
 
+    fn will_change(&mut self, a: bool, b: bool) -> bool {
+        // if either side changed from what it was, that could cause equality to fail
+        a || b
+    }
+
     fn relate_with_variance<T:Relate<'a,'tcx>>(&mut self,
                                                _: ty::Variance,
                                                a: &T,

src/librustc/middle/infer/error_reporting.rs

@@ -593,7 +593,8 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
                                sub: Region,
                                sup: Region) {
         match origin {
-            infer::Subtype(trace) => {
+            infer::Subtype(trace) |
+            infer::DefaultExistentialBound(trace) => {
                 let terr = ty::terr_regions_does_not_outlive(sup, sub);
                 self.report_and_explain_type_error(trace, &terr);
             }
@@ -1569,7 +1570,8 @@ impl<'a, 'tcx> ErrorReportingHelpers<'tcx> for InferCtxt<'a, 'tcx> {
 
     fn note_region_origin(&self, origin: &SubregionOrigin<'tcx>) {
         match *origin {
-            infer::Subtype(ref trace) => {
+            infer::Subtype(ref trace) |
+            infer::DefaultExistentialBound(ref trace) => {
                 let desc = match trace.origin {
                     infer::Misc(_) => {
                         "types are compatible"

src/librustc/middle/infer/glb.rs

@@ -35,6 +35,16 @@ impl<'a, 'tcx> TypeRelation<'a, 'tcx> for Glb<'a, 'tcx> {
 
     fn a_is_expected(&self) -> bool { self.fields.a_is_expected }
 
+    fn will_change(&mut self, a: bool, b: bool) -> bool {
+        // Hmm, so the result of GLB will still be a LB if one or both
+        // sides change to 'static, but it may no longer be the GLB.
+        // I'm going to go with `a || b` here to be conservative,
+        // since the result of this operation may be affected, though
+        // I think it would mostly be more accepting than before (since the result
+        // would be a bigger region).
+        a || b
+    }
+
     fn relate_with_variance<T:Relate<'a,'tcx>>(&mut self,
                                                variance: ty::Variance,
                                                a: &T,

src/librustc/middle/infer/lub.rs

@@ -35,6 +35,11 @@ impl<'a, 'tcx> TypeRelation<'a, 'tcx> for Lub<'a, 'tcx> {
 
     fn a_is_expected(&self) -> bool { self.fields.a_is_expected }
 
+    fn will_change(&mut self, a: bool, b: bool) -> bool {
+        // result will be 'static if a || b
+        a || b
+    }
+
     fn relate_with_variance<T:Relate<'a,'tcx>>(&mut self,
                                                variance: ty::Variance,
                                                a: &T,

src/librustc/middle/infer/mod.rs

@@ -194,6 +194,9 @@ pub enum SubregionOrigin<'tcx> {
     // Arose from a subtyping relation
     Subtype(TypeTrace<'tcx>),
 
+    // Arose from a subtyping relation
+    DefaultExistentialBound(TypeTrace<'tcx>),
+
     // Stack-allocated closures cannot outlive innermost loop
     // or function so as to ensure we only require finite stack
     InfStackClosure(Span),
@@ -658,7 +661,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                       -> CombineFields<'a, 'tcx> {
         CombineFields {infcx: self,
                        a_is_expected: a_is_expected,
-                       trace: trace}
+                       trace: trace,
+                       cause: None}
     }
 
     // public so that it can be used from the rustc_driver unit tests
@@ -1464,6 +1468,7 @@ impl<'tcx> SubregionOrigin<'tcx> {
     pub fn span(&self) -> Span {
         match *self {
             Subtype(ref a) => a.span(),
+            DefaultExistentialBound(ref a) => a.span(),
             InfStackClosure(a) => a,
             InvokeClosure(a) => a,
             DerefPointer(a) => a,

src/librustc/middle/infer/region_inference/mod.rs

@@ -1358,9 +1358,56 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> {
             }
         }
 
+        // Check for future hostile edges tied to a bad default
+        self.report_future_hostility(&graph);
+
         (0..self.num_vars() as usize).map(|idx| var_data[idx].value).collect()
     }
 
+    fn report_future_hostility(&self, graph: &RegionGraph) {
+        let constraints = self.constraints.borrow();
+        for edge in graph.all_edges() {
+            match constraints[&edge.data] {
+                SubregionOrigin::DefaultExistentialBound(_) => {
+                    // this will become 'static in the future
+                }
+                _ => { continue; }
+            }
+
+            // this constraint will become a 'static constraint in the
+            // future, so walk outward and see if we have any hard
+            // bounds that could not be inferred to 'static
+            for nid in graph.depth_traverse(edge.target()) {
+                for (_, succ) in graph.outgoing_edges(nid) {
+                    match succ.data {
+                        ConstrainVarSubReg(_, r) => {
+                            match r {
+                                ty::ReStatic | ty::ReInfer(_) => {
+                                    /* OK */
+                                }
+                                ty::ReFree(_) | ty::ReScope(_) | ty::ReEmpty => {
+                                    span_warn!(
+                                        self.tcx.sess,
+                                        constraints[&edge.data].span(),
+                                        E0398,
+                                        "this code may fail to compile in Rust 1.3 due to \
+                                         the proposed change in object lifetime bound defaults");
+                                    return; // only issue the warning once per fn
+                                }
+                                ty::ReEarlyBound(..) | ty::ReLateBound(..) => {
+                                    self.tcx.sess.span_bug(
+                                        constraints[&succ.data].span(),
+                                        "relation to bound region");
+                                }
+                            }
+                        }
+                        _ => { }
+                    }
+                }
+            }
+        }
+    }
+
     fn construct_graph(&self) -> RegionGraph {
         let num_vars = self.num_vars();
 

src/librustc/middle/infer/sub.rs

@@ -10,16 +10,17 @@
 
 use super::combine::{self, CombineFields};
 use super::higher_ranked::HigherRankedRelations;
-use super::Subtype;
+use super::SubregionOrigin;
 use super::type_variable::{SubtypeOf, SupertypeOf};
 
 use middle::ty::{self, Ty};
 use middle::ty::TyVar;
-use middle::ty_relate::{Relate, RelateResult, TypeRelation};
+use middle::ty_relate::{Cause, Relate, RelateResult, TypeRelation};
+use std::mem;
 
 /// "Greatest lower bound" (common subtype)
 pub struct Sub<'a, 'tcx: 'a> {
-    fields: CombineFields<'a, 'tcx>
+    fields: CombineFields<'a, 'tcx>,
 }
 
 impl<'a, 'tcx> Sub<'a, 'tcx> {
@@ -33,6 +34,25 @@ impl<'a, 'tcx> TypeRelation<'a, 'tcx> for Sub<'a, 'tcx> {
     fn tcx(&self) -> &'a ty::ctxt<'tcx> { self.fields.infcx.tcx }
     fn a_is_expected(&self) -> bool { self.fields.a_is_expected }
 
+    fn with_cause<F,R>(&mut self, cause: Cause, f: F) -> R
+        where F: FnOnce(&mut Self) -> R
+    {
+        debug!("sub with_cause={:?}", cause);
+        let old_cause = mem::replace(&mut self.fields.cause, Some(cause));
+        let r = f(self);
+        debug!("sub old_cause={:?}", old_cause);
+        self.fields.cause = old_cause;
+        r
+    }
+
+    fn will_change(&mut self, a: bool, b: bool) -> bool {
+        // if we have (Foo+'a) <: (Foo+'b), this requires that 'a:'b.
+        // So if 'a becomes 'static, no additional errors can occur.
+        // OTOH, if 'a stays the same, but 'b becomes 'static, we
+        // could have a problem.
+        !a && b
+    }
+
     fn relate_with_variance<T:Relate<'a,'tcx>>(&mut self,
                                                variance: ty::Variance,
                                                a: &T,
@@ -84,11 +104,14 @@ impl<'a, 'tcx> TypeRelation<'a, 'tcx> for Sub<'a, 'tcx> {
     }
 
     fn regions(&mut self, a: ty::Region, b: ty::Region) -> RelateResult<'tcx, ty::Region> {
-        debug!("{}.regions({:?}, {:?})",
-               self.tag(),
-               a,
-               b);
-        let origin = Subtype(self.fields.trace.clone());
+        debug!("{}.regions({:?}, {:?}) self.cause={:?}",
+               self.tag(), a, b, self.fields.cause);
+        let origin = match self.fields.cause {
+            Some(Cause::ExistentialRegionBound(true)) =>
+                SubregionOrigin::DefaultExistentialBound(self.fields.trace.clone()),
+            _ =>
+                SubregionOrigin::Subtype(self.fields.trace.clone()),
+        };
         self.fields.infcx.region_vars.make_subregion(origin, a, b);
         Ok(a)
     }

src/librustc/middle/traits/select.rs

@@ -2445,6 +2445,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     region_bound: data_b.bounds.region_bound,
                     builtin_bounds: data_b.bounds.builtin_bounds,
                     projection_bounds: data_a.bounds.projection_bounds.clone(),
+                    region_bound_will_change: data_b.bounds.region_bound_will_change,
                 };
 
                 let new_trait = tcx.mk_trait(data_a.principal.clone(), bounds);

src/librustc/middle/ty.rs

@@ -2055,6 +2055,11 @@ pub struct ExistentialBounds<'tcx> {
     pub region_bound: ty::Region,
     pub builtin_bounds: BuiltinBounds,
     pub projection_bounds: Vec<PolyProjectionPredicate<'tcx>>,
+
+    // If true, this TyTrait used a "default bound" in the surface
+    // syntax.  This makes no difference to the type system but is
+    // handy for error reporting.
+    pub region_bound_will_change: bool,
 }
 
 #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
@@ -2245,6 +2250,9 @@ pub enum ObjectLifetimeDefault {
     /// `T:'a` constraints are found.
     Ambiguous,
 
+    /// Use the base default, typically 'static, but in a fn body it is a fresh variable
+    BaseDefault,
+
     /// Use the given region as the default.
     Specific(Region),
 }
@@ -2256,7 +2264,7 @@ pub struct TypeParameterDef<'tcx> {
     pub space: subst::ParamSpace,
     pub index: u32,
     pub default: Option<Ty<'tcx>>,
-    pub object_lifetime_default: Option<ObjectLifetimeDefault>,
+    pub object_lifetime_default: ObjectLifetimeDefault,
 }
 
 #[derive(RustcEncodable, RustcDecodable, Clone, Debug)]
@@ -7328,6 +7336,7 @@ impl<'tcx> fmt::Debug for ObjectLifetimeDefault {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
             ObjectLifetimeDefault::Ambiguous => write!(f, "Ambiguous"),
+            ObjectLifetimeDefault::BaseDefault => write!(f, "BaseDefault"),
             ObjectLifetimeDefault::Specific(ref r) => write!(f, "{:?}", r),
         }
     }