From 8122ce81d0909bed39c6df3e47bc751851bded84 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Thu, 5 Feb 2015 21:46:01 +1300 Subject: [PATCH 1/2] Accept quantification of lifetimes outside the self type in where clauses. Closes #20022 --- src/librustc/diagnostics.rs | 3 +- src/librustc/middle/infer/error_reporting.rs | 3 +- src/librustc/middle/resolve_lifetime.rs | 64 ++++++++++++++++---- src/librustc/middle/ty.rs | 2 +- src/librustc_typeck/astconv.rs | 2 +- src/libsyntax/ast.rs | 3 + src/libsyntax/ext/build.rs | 9 +-- src/libsyntax/ext/deriving/generic/mod.rs | 1 + src/libsyntax/fold.rs | 7 ++- src/libsyntax/parse/parser.rs | 33 ++++++++-- 10 files changed, 100 insertions(+), 27 deletions(-) diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs index a808b417b4c6b..efcc3715d3e3d 100644 --- a/src/librustc/diagnostics.rs +++ b/src/librustc/diagnostics.rs @@ -126,7 +126,8 @@ register_diagnostics! { E0312, // lifetime of reference outlives lifetime of borrowed content E0313, // lifetime of borrowed pointer outlives lifetime of captured variable E0314, // closure outlives stack frame - E0315 // cannot invoke closure outside of its lifetime + E0315, // cannot invoke closure outside of its lifetime + E0316 // nested quantification of lifetimes } __build_diagnostic_array! { DIAGNOSTICS } diff --git a/src/librustc/middle/infer/error_reporting.rs b/src/librustc/middle/infer/error_reporting.rs index 1b513a13588c9..99896535442d8 100644 --- a/src/librustc/middle/infer/error_reporting.rs +++ b/src/librustc/middle/infer/error_reporting.rs @@ -1076,7 +1076,8 @@ impl<'a, 'tcx> Rebuilder<'a, 'tcx> { trait_ref: ast::TraitRef { path: new_path, ref_id: tr.ref_id, - } + }, + span: poly_tr.span, }, modifier) } } diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index 5e27023e026fb..6f38dc8106420 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -45,8 +45,8 @@ pub enum DefRegion { /* lifetime decl */ ast::NodeId), } -// maps the id of each lifetime reference to the lifetime decl -// that it corresponds to +// Maps the id of each lifetime reference to the lifetime decl +// that it corresponds to. pub type NamedRegionMap = NodeMap; struct LifetimeContext<'a> { @@ -54,6 +54,22 @@ struct LifetimeContext<'a> { named_region_map: &'a mut NamedRegionMap, scope: Scope<'a>, def_map: &'a DefMap, + // Deep breath. Our representation for poly trait refs contains a single + // binder and thus we only allow a single level of quantification. However, + // the syntax of Rust permits quantification in two places, e.g., `T: for <'a> Foo<'a>` + // and `for <'a, 'b> &'b T: Foo<'a>`. In order to get the de Bruijn indices + // correct when representing these constraints, we should only introduce one + // scope. However, we want to support both locations for the quantifier and + // during lifetime resolution we want precise information (so we can't + // desugar in an earlier phase). + + // SO, if we encounter a quantifier at the outer scope, we set + // trait_ref_hack to true (and introduce a scope), and then if we encounter + // a quantifier at the inner scope, we error. If trait_ref_hack is false, + // then we introduce the scope at the inner quantifier. + + // I'm sorry. + trait_ref_hack: bool, } enum ScopeChain<'a> { @@ -80,6 +96,7 @@ pub fn krate(sess: &Session, krate: &ast::Crate, def_map: &DefMap) -> NamedRegio named_region_map: &mut named_region_map, scope: &ROOT_SCOPE, def_map: def_map, + trait_ref_hack: false, }, krate); sess.abort_if_errors(); named_region_map @@ -198,9 +215,22 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> { match predicate { &ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate{ ref bounded_ty, ref bounds, + ref bound_lifetimes, .. }) => { - self.visit_ty(&**bounded_ty); - visit::walk_ty_param_bounds_helper(self, bounds); + if bound_lifetimes.len() > 0 { + self.trait_ref_hack = true; + let result = self.with(LateScope(bound_lifetimes, self.scope), + |old_scope, this| { + this.check_lifetime_defs(old_scope, bound_lifetimes); + this.visit_ty(&**bounded_ty); + visit::walk_ty_param_bounds_helper(this, bounds); + }); + self.trait_ref_hack = false; + result + } else { + self.visit_ty(&**bounded_ty); + visit::walk_ty_param_bounds_helper(self, bounds); + } } &ast::WherePredicate::RegionPredicate(ast::WhereRegionPredicate{ref lifetime, ref bounds, @@ -222,18 +252,27 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> { } } - fn visit_poly_trait_ref(&mut self, trait_ref: - &ast::PolyTraitRef, + fn visit_poly_trait_ref(&mut self, + trait_ref: &ast::PolyTraitRef, _modifier: &ast::TraitBoundModifier) { debug!("visit_poly_trait_ref trait_ref={:?}", trait_ref); - self.with(LateScope(&trait_ref.bound_lifetimes, self.scope), |old_scope, this| { - this.check_lifetime_defs(old_scope, &trait_ref.bound_lifetimes); - for lifetime in &trait_ref.bound_lifetimes { - this.visit_lifetime_def(lifetime); + if !self.trait_ref_hack || trait_ref.bound_lifetimes.len() > 0 { + if self.trait_ref_hack { + println!("{:?}", trait_ref.span); + span_err!(self.sess, trait_ref.span, E0316, + "nested quantification of lifetimes"); } - this.visit_trait_ref(&trait_ref.trait_ref) - }) + self.with(LateScope(&trait_ref.bound_lifetimes, self.scope), |old_scope, this| { + this.check_lifetime_defs(old_scope, &trait_ref.bound_lifetimes); + for lifetime in &trait_ref.bound_lifetimes { + this.visit_lifetime_def(lifetime); + } + this.visit_trait_ref(&trait_ref.trait_ref) + }) + } else { + self.visit_trait_ref(&trait_ref.trait_ref) + } } fn visit_trait_ref(&mut self, trait_ref: &ast::TraitRef) { @@ -251,6 +290,7 @@ impl<'a> LifetimeContext<'a> { named_region_map: *named_region_map, scope: &wrap_scope, def_map: self.def_map, + trait_ref_hack: self.trait_ref_hack, }; debug!("entering scope {:?}", this.scope); f(self.scope, &mut this); diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index b29a23924bb12..95f7ff5852440 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -1478,7 +1478,7 @@ impl<'tcx> PolyTraitRef<'tcx> { /// compiler's representation for things like `for<'a> Fn(&'a int)` /// (which would be represented by the type `PolyTraitRef == /// Binder`). Note that when we skolemize, instantiate, -/// erase, or otherwise "discharge" these bound reons, we change the +/// erase, or otherwise "discharge" these bound regions, we change the /// type from `Binder` to just `T` (see /// e.g. `liberate_late_bound_regions`). #[derive(Clone, PartialEq, Eq, Hash, Debug)] diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 60771d126b51b..0dbb53e09b7d9 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -526,7 +526,7 @@ pub fn instantiate_poly_trait_ref<'tcx>( { let mut projections = Vec::new(); - // the trait reference introduces a binding level here, so + // The trait reference introduces a binding level here, so // we need to shift the `rscope`. It'd be nice if we could // do away with this rscope stuff and work this knowledge // into resolve_lifetimes, as we do with non-omitted diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 71259ff5d9ade..f8793f81b1996 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -443,6 +443,7 @@ pub enum WherePredicate { #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)] pub struct WhereBoundPredicate { pub span: Span, + pub bound_lifetimes: Vec, pub bounded_ty: P, pub bounds: OwnedSlice, } @@ -1535,6 +1536,8 @@ pub struct PolyTraitRef { /// The `Foo<&'a T>` in `<'a> Foo<&'a T>` pub trait_ref: TraitRef, + + pub span: Span, } #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)] diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index a7d1baf08beac..6d9a2fdb9f198 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -70,7 +70,7 @@ pub trait AstBuilder { default: Option>) -> ast::TyParam; fn trait_ref(&self, path: ast::Path) -> ast::TraitRef; - fn poly_trait_ref(&self, path: ast::Path) -> ast::PolyTraitRef; + fn poly_trait_ref(&self, span: Span, path: ast::Path) -> ast::PolyTraitRef; fn typarambound(&self, path: ast::Path) -> ast::TyParamBound; fn lifetime(&self, span: Span, ident: ast::Name) -> ast::Lifetime; fn lifetime_def(&self, @@ -442,15 +442,16 @@ impl<'a> AstBuilder for ExtCtxt<'a> { } } - fn poly_trait_ref(&self, path: ast::Path) -> ast::PolyTraitRef { + fn poly_trait_ref(&self, span: Span, path: ast::Path) -> ast::PolyTraitRef { ast::PolyTraitRef { bound_lifetimes: Vec::new(), - trait_ref: self.trait_ref(path) + trait_ref: self.trait_ref(path), + span: span, } } fn typarambound(&self, path: ast::Path) -> ast::TyParamBound { - ast::TraitTyParamBound(self.poly_trait_ref(path), ast::TraitBoundModifier::None) + ast::TraitTyParamBound(self.poly_trait_ref(path.span, path), ast::TraitBoundModifier::None) } fn lifetime(&self, span: Span, name: ast::Name) -> ast::Lifetime { diff --git a/src/libsyntax/ext/deriving/generic/mod.rs b/src/libsyntax/ext/deriving/generic/mod.rs index 28573ef757b12..d9242417e0475 100644 --- a/src/libsyntax/ext/deriving/generic/mod.rs +++ b/src/libsyntax/ext/deriving/generic/mod.rs @@ -443,6 +443,7 @@ impl<'a> TraitDef<'a> { ast::WherePredicate::BoundPredicate(ref wb) => { ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate { span: self.span, + bound_lifetimes: wb.bound_lifetimes.clone(), bounded_ty: wb.bounded_ty.clone(), bounds: OwnedSlice::from_vec(wb.bounds.iter().map(|b| b.clone()).collect()) }) diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index b0ddb655882a8..8f1d15f6da885 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -806,10 +806,12 @@ pub fn noop_fold_where_predicate( fld: &mut T) -> WherePredicate { match pred { - ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate{bounded_ty, + ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate{bound_lifetimes, + bounded_ty, bounds, span}) => { ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate { + bound_lifetimes: fld.fold_lifetime_defs(bound_lifetimes), bounded_ty: fld.fold_ty(bounded_ty), bounds: bounds.move_map(|x| fld.fold_ty_param_bound(x)), span: fld.new_span(span) @@ -895,7 +897,8 @@ pub fn noop_fold_trait_ref(p: TraitRef, fld: &mut T) -> TraitRef { pub fn noop_fold_poly_trait_ref(p: PolyTraitRef, fld: &mut T) -> PolyTraitRef { ast::PolyTraitRef { bound_lifetimes: fld.fold_lifetime_defs(p.bound_lifetimes), - trait_ref: fld.fold_trait_ref(p.trait_ref) + trait_ref: fld.fold_trait_ref(p.trait_ref), + span: fld.new_span(p.span), } } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 3107f47de7859..fd2f0685cab83 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1036,6 +1036,8 @@ impl<'a> Parser<'a> { */ // parse <'lt> + let lo = self.span.lo; + let lifetime_defs = self.parse_late_bound_lifetime_defs(); // examine next token to decide to do @@ -1047,9 +1049,11 @@ impl<'a> Parser<'a> { self.token.is_ident() || self.token.is_path() { + let hi = self.span.hi; let trait_ref = self.parse_trait_ref(); let poly_trait_ref = ast::PolyTraitRef { bound_lifetimes: lifetime_defs, - trait_ref: trait_ref }; + trait_ref: trait_ref, + span: mk_sp(lo, hi)}; let other_bounds = if self.eat(&token::BinOp(token::Plus)) { self.parse_ty_param_bounds(BoundParsingMode::Bare) } else { @@ -4070,7 +4074,8 @@ impl<'a> Parser<'a> { if let Some(unbound) = unbound { let mut bounds_as_vec = bounds.into_vec(); bounds_as_vec.push(TraitTyParamBound(PolyTraitRef { bound_lifetimes: vec![], - trait_ref: unbound }, + trait_ref: unbound, + span: span }, TraitBoundModifier::Maybe)); bounds = OwnedSlice::from_vec(bounds_as_vec); }; @@ -4223,6 +4228,16 @@ impl<'a> Parser<'a> { } _ => { + let bound_lifetimes = if self.eat_keyword(keywords::For) { + // Higher ranked constraint. + self.expect(&token::Lt); + let lifetime_defs = self.parse_lifetime_defs(); + self.expect_gt(); + lifetime_defs + } else { + vec![] + }; + let bounded_ty = self.parse_ty(); if self.eat(&token::Colon) { @@ -4233,12 +4248,13 @@ impl<'a> Parser<'a> { if bounds.len() == 0 { self.span_err(span, "each predicate in a `where` clause must have \ - at least one bound in it"); + at least one bound in it"); } generics.where_clause.predicates.push(ast::WherePredicate::BoundPredicate( ast::WhereBoundPredicate { span: span, + bound_lifetimes: bound_lifetimes, bounded_ty: bounded_ty, bounds: bounds, })); @@ -4674,8 +4690,12 @@ impl<'a> Parser<'a> { /// Parse trait Foo { ... } fn parse_item_trait(&mut self, unsafety: Unsafety) -> ItemInfo { + let ident = self.parse_ident(); let mut tps = self.parse_generics(); + // This is not very accurate, but since unbound only exists to catch + // obsolete syntax, the span is unlikely to ever be used. + let unbound_span = self.span; let unbound = self.parse_for_sized(); // Parse supertrait bounds. @@ -4684,7 +4704,8 @@ impl<'a> Parser<'a> { if let Some(unbound) = unbound { let mut bounds_as_vec = bounds.into_vec(); bounds_as_vec.push(TraitTyParamBound(PolyTraitRef { bound_lifetimes: vec![], - trait_ref: unbound }, + trait_ref: unbound, + span: unbound_span }, TraitBoundModifier::Maybe)); bounds = OwnedSlice::from_vec(bounds_as_vec); }; @@ -4803,11 +4824,13 @@ impl<'a> Parser<'a> { /// Parse for<'l> a::B fn parse_poly_trait_ref(&mut self) -> PolyTraitRef { + let lo = self.span.lo; let lifetime_defs = self.parse_late_bound_lifetime_defs(); ast::PolyTraitRef { bound_lifetimes: lifetime_defs, - trait_ref: self.parse_trait_ref() + trait_ref: self.parse_trait_ref(), + span: mk_sp(lo, self.last_span.hi), } } From f9c577e5141dda6413efcfd036389d7d2480e528 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Mon, 9 Feb 2015 16:49:27 +1300 Subject: [PATCH 2/2] Tests --- src/libsyntax/print/pprust.rs | 14 ++++-- src/test/compile-fail/where-for-self-2.rs | 33 +++++++++++++ src/test/compile-fail/where-for-self.rs | 29 +++++++++++ src/test/run-pass/where-for-self.rs | 59 +++++++++++++++++++++++ 4 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 src/test/compile-fail/where-for-self-2.rs create mode 100644 src/test/compile-fail/where-for-self.rs create mode 100644 src/test/run-pass/where-for-self.rs diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 0da15859ea2df..583095e157427 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -1025,11 +1025,11 @@ impl<'a> State<'a> { self.print_path(&t.path, false) } - fn print_poly_trait_ref(&mut self, t: &ast::PolyTraitRef) -> IoResult<()> { - if !t.bound_lifetimes.is_empty() { + fn print_formal_lifetime_list(&mut self, lifetimes: &[ast::LifetimeDef]) -> IoResult<()> { + if !lifetimes.is_empty() { try!(word(&mut self.s, "for<")); let mut comma = false; - for lifetime_def in &t.bound_lifetimes { + for lifetime_def in lifetimes { if comma { try!(self.word_space(",")) } @@ -1038,7 +1038,11 @@ impl<'a> State<'a> { } try!(word(&mut self.s, ">")); } + Ok(()) + } + fn print_poly_trait_ref(&mut self, t: &ast::PolyTraitRef) -> IoResult<()> { + try!(self.print_formal_lifetime_list(&t.bound_lifetimes)); self.print_trait_ref(&t.trait_ref) } @@ -2517,9 +2521,11 @@ impl<'a> State<'a> { } match predicate { - &ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate{ref bounded_ty, + &ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate{ref bound_lifetimes, + ref bounded_ty, ref bounds, ..}) => { + try!(self.print_formal_lifetime_list(bound_lifetimes)); try!(self.print_type(&**bounded_ty)); try!(self.print_bounds(":", bounds)); } diff --git a/src/test/compile-fail/where-for-self-2.rs b/src/test/compile-fail/where-for-self-2.rs new file mode 100644 index 0000000000000..cd5240198b385 --- /dev/null +++ b/src/test/compile-fail/where-for-self-2.rs @@ -0,0 +1,33 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that we can quantify lifetimes outside a constraint (i.e., including +// the self type) in a where clause. Specifically, test that implementing for a +// specific lifetime is not enough to satisify the `for<'a> ...` constraint, which +// should require *all* lifetimes. + +static X: &'static u32 = &42; + +trait Bar { + fn bar(&self); +} + +impl Bar for &'static u32 { + fn bar(&self) {} +} + +fn foo(x: &T) + where for<'a> &'a T: Bar +{} + +fn main() { + foo(&X); + //~^ error: `for<'a> Bar` is not implemented +} diff --git a/src/test/compile-fail/where-for-self.rs b/src/test/compile-fail/where-for-self.rs new file mode 100644 index 0000000000000..8f447face4e51 --- /dev/null +++ b/src/test/compile-fail/where-for-self.rs @@ -0,0 +1,29 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that we can quantify lifetimes outside a constraint (i.e., including +// the self type) in a where clause. Specifically, test that we cannot nest +// quantification in constraints (to be clear, there is no reason this should not +// we're testing we don't crash or do something stupid). + +trait Bar<'a> { + fn bar(&self); +} + +impl<'a, 'b> Bar<'b> for &'a u32 { + fn bar(&self) {} +} + +fn foo(x: &T) + where for<'a> &'a T: for<'b> Bar<'b> + //~^ error: nested quantification of lifetimes +{} + +fn main() {} diff --git a/src/test/run-pass/where-for-self.rs b/src/test/run-pass/where-for-self.rs new file mode 100644 index 0000000000000..5d426793c2e36 --- /dev/null +++ b/src/test/run-pass/where-for-self.rs @@ -0,0 +1,59 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that we can quantify lifetimes outside a constraint (i.e., including +// the self type) in a where clause. + +static mut COUNT: u32 = 1; + +trait Bar<'a> { + fn bar(&self); +} + +trait Baz<'a> { + fn baz(&self); +} + +impl<'a, 'b> Bar<'b> for &'a u32 { + fn bar(&self) { + unsafe { COUNT *= 2; } + } +} + +impl<'a, 'b> Baz<'b> for &'a u32 { + fn baz(&self) { + unsafe { COUNT *= 3; } + } +} + +// Test we can use the syntax for HRL including the self type. +fn foo1(x: &T) + where for<'a, 'b> &'a T: Bar<'b> +{ + x.bar() +} + +// Test we can quantify multiple bounds (i.e., the precedence is sensible). +fn foo2(x: &T) + where for<'a, 'b> &'a T: Bar<'b> + Baz<'b> +{ + x.baz(); + x.bar() +} + +fn main() { + let x = 42u32; + foo1(&x); + foo2(&x); + unsafe { + assert!(COUNT == 12); + } +} +