Skip to content

Commit

Permalink
rustc: fixed regionck for owned trait casts.
Browse files Browse the repository at this point in the history
Regionck didn't do any checks for casts/coercions to an owned trait,
which resulted in lifetimes of the source pointer
to be ignored in the result of such cast.

This fix constraints all regions of the source type of the cast/coercion to be superregions
of each region of the target type (if target trait definition has some lifetime params),
or of 'static lifetime (if there're no lifetime params in target trait's definition).

Closes rust-lang#5723
Closes rust-lang#9745
Closes rust-lang#11971
  • Loading branch information
dmski committed Apr 8, 2014
1 parent e415c25 commit 4d77945
Show file tree
Hide file tree
Showing 10 changed files with 181 additions and 45 deletions.
85 changes: 61 additions & 24 deletions src/librustc/middle/typeck/check/regionck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,24 +419,9 @@ fn visit_expr(rcx: &mut Rcx, expr: &ast::Expr) {
infer::AutoBorrow(expr.span));
}
}
ty::AutoObject(ast::BorrowedSigil, Some(trait_region), _, _, _, _) => {
// Determine if we are casting `expr` to an trait
// instance. If so, we have to be sure that the type of
// the source obeys the trait's region bound.
//
// Note: there is a subtle point here concerning type
// parameters. It is possible that the type of `source`
// contains type parameters, which in turn may contain
// regions that are not visible to us (only the caller
// knows about them). The kind checker is ultimately
// responsible for guaranteeing region safety in that
// particular case. There is an extensive comment on the
// function check_cast_for_escaping_regions() in kind.rs
// explaining how it goes about doing that.

let source_ty = rcx.fcx.expr_ty(expr);
constrain_regions_in_type(rcx, trait_region,
infer::RelateObjectBound(expr.span), source_ty);
ty::AutoObject(_, maybe_region, _, _, _, ref substs) => {
let source_ty = rcx.resolve_node_type(expr.id);
constrain_trait_cast(rcx, expr, maybe_region, substs, source_ty);
}
_ => {}
}
Expand Down Expand Up @@ -540,13 +525,15 @@ fn visit_expr(rcx: &mut Rcx, expr: &ast::Expr) {
// explaining how it goes about doing that.
let target_ty = rcx.resolve_node_type(expr.id);
match ty::get(target_ty).sty {
ty::ty_trait(~ty::TyTrait { store: ty::RegionTraitStore(trait_region), .. }) => {
ty::ty_trait(~ty::TyTrait { store: trait_store, substs: ref substs, .. }) => {
let source_ty = rcx.resolve_expr_type_adjusted(source);
constrain_regions_in_type(
rcx,
trait_region,
infer::RelateObjectBound(expr.span),
source_ty);

let trait_region = match trait_store {
ty::RegionTraitStore(r) => Some(r),
ty::UniqTraitStore => None
};

constrain_trait_cast(rcx, expr, trait_region, substs, source_ty);
}
_ => ()
}
Expand Down Expand Up @@ -933,6 +920,56 @@ fn constrain_index(rcx: &mut Rcx,
}
}

fn constrain_trait_cast(rcx: &mut Rcx,
expr: &ast::Expr,
trait_region: Option<ty::Region>,
trait_substs: &ty::substs,
source_ty: ty::t) {
// If we are casting `source` to a trait
// instance, we have to be sure that the type of
// the source obeys the trait's region bound.
//
// Note: there is a subtle point here concerning type
// parameters. It is possible that the type of `source`
// contains type parameters, which in turn may contain
// regions that are not visible to us (only the caller
// knows about them). The kind checker is ultimately
// responsible for guaranteeing region safety in that
// particular case. There is an extensive comment on the
// function check_cast_for_escaping_regions() in kind.rs
// explaining how it goes about doing that.

debug!("constrain_trait_cast(expr={:?}, trait_region={:?}, trait_substs={:?}, source_ty={:?}",
expr, trait_region, trait_substs, ty::get(source_ty));

let mut regions = Vec::new();

match trait_substs.regions {
ty::NonerasedRegions(ref regs) => {
for r in regs.iter() {
regions.push(*r);
}
}
ty::ErasedRegions => ()
}

match trait_region {
Some(region) => {
regions.push(region);
}
None => {
// casting to owned trait with no lifetime params in trait def
if regions.is_empty() {
regions.push(ty::ReStatic);
}
}
}

for r in regions.iter() {
constrain_regions_in_type(rcx, *r, infer::RelateObjectBound(expr.span), source_ty);
}
}

fn constrain_regions_in_type_of_node(
rcx: &mut Rcx,
id: ast::NodeId,
Expand Down
12 changes: 6 additions & 6 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,20 +98,20 @@ pub type IdentMacroExpanderFn =
pub type MacroCrateRegistrationFun =
fn(|ast::Name, SyntaxExtension|);

pub trait AnyMacro {
pub trait AnyMacro<'a> {
fn make_expr(&self) -> @ast::Expr;
fn make_items(&self) -> SmallVector<@ast::Item>;
fn make_stmt(&self) -> @ast::Stmt;
}


pub enum MacResult {
pub enum MacResult<'a> {
MRExpr(@ast::Expr),
MRItem(@ast::Item),
MRAny(~AnyMacro:),
MRAny(~AnyMacro<'a>:),
MRDef(MacroDef),
}
impl MacResult {
impl<'a> MacResult<'a> {
/// Create an empty expression MacResult; useful for satisfying
/// type signatures after emitting a non-fatal error (which stop
/// compilation well before the validity (or otherwise)) of the
Expand All @@ -123,7 +123,7 @@ impl MacResult {
span: sp,
}
}
pub fn dummy_expr(sp: codemap::Span) -> MacResult {
pub fn dummy_expr(sp: codemap::Span) -> MacResult<'a> {
MRExpr(MacResult::raw_dummy_expr(sp))
}
pub fn dummy_any(sp: codemap::Span) -> MacResult {
Expand All @@ -133,7 +133,7 @@ impl MacResult {
struct DummyMacResult {
sp: codemap::Span
}
impl AnyMacro for DummyMacResult {
impl<'a> AnyMacro<'a> for DummyMacResult {
fn make_expr(&self) -> @ast::Expr {
MacResult::raw_dummy_expr(self.sp)
}
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/ext/tt/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl<'a> ParserAnyMacro<'a> {
}
}

impl<'a> AnyMacro for ParserAnyMacro<'a> {
impl<'a> AnyMacro<'a> for ParserAnyMacro<'a> {
fn make_expr(&self) -> @ast::Expr {
let ret = self.parser.borrow_mut().parse_expr();
self.ensure_complete_parse(true);
Expand Down Expand Up @@ -106,7 +106,7 @@ impl MacroExpander for MacroRulesMacroExpander {
}

// Given `lhses` and `rhses`, this is the new macro we create
fn generic_extension(cx: &ExtCtxt,
fn generic_extension<'a>(cx: &'a ExtCtxt,
sp: Span,
name: Ident,
arg: &[ast::TokenTree],
Expand Down
6 changes: 3 additions & 3 deletions src/libsyntax/parse/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use std::str;

pub use ext::tt::transcribe::{TtReader, new_tt_reader};

pub trait Reader {
pub trait Reader<'a> {
fn is_eof(&self) -> bool;
fn next_token(&mut self) -> TokenAndSpan;
fn fatal(&self, ~str) -> !;
Expand Down Expand Up @@ -89,7 +89,7 @@ pub fn new_low_level_string_reader<'a>(span_diagnostic: &'a SpanHandler,
r
}

impl<'a> Reader for StringReader<'a> {
impl<'a> Reader<'a> for StringReader<'a> {
fn is_eof(&self) -> bool { is_eof(self) }
// return the next token. EFFECT: advances the string_reader.
fn next_token(&mut self) -> TokenAndSpan {
Expand All @@ -113,7 +113,7 @@ impl<'a> Reader for StringReader<'a> {
}
}

impl<'a> Reader for TtReader<'a> {
impl<'a> Reader<'a> for TtReader<'a> {
fn is_eof(&self) -> bool {
self.cur_tok == token::EOF
}
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ struct ParsedItemsAndViewItems {

/* ident is handled by common.rs */

pub fn Parser<'a>(sess: &'a ParseSess, cfg: ast::CrateConfig, mut rdr: ~Reader:)
pub fn Parser<'a>(sess: &'a ParseSess, cfg: ast::CrateConfig, mut rdr: ~Reader<'a>:)
-> Parser<'a> {
let tok0 = rdr.next_token();
let span = tok0.sp;
Expand Down Expand Up @@ -324,7 +324,7 @@ pub struct Parser<'a> {
pub tokens_consumed: uint,
pub restriction: restriction,
pub quote_depth: uint, // not (yet) related to the quasiquoter
pub reader: ~Reader:,
pub reader: ~Reader<'a>:,
pub interner: Rc<token::IdentInterner>,
/// The set of seen errors about obsolete syntax. Used to suppress
/// extra detail when the same error is seen twice
Expand Down
16 changes: 8 additions & 8 deletions src/test/compile-fail/owned-ptr-static-bound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@
trait A<T> {}
struct B<'a, T>(&'a A<T>);

trait X {}
impl<'a, T> X for B<'a, T> {}
trait X<'a> {}
impl<'a, T> X<'a> for B<'a, T> {}

fn f<'a, T, U>(v: ~A<T>) -> ~X: {
~B(v) as ~X: //~ ERROR value may contain references; add `'static` bound to `T`
fn f<'a, T, U>(v: &'a A<T>) -> ~X<'a>: {
~B(v) as ~X<'a>: //~ ERROR value may contain references; add `'static` bound to `T`
}

fn g<'a, T, U>(v: ~A<U>) -> ~X: {
~B(v) as ~X: //~ ERROR value may contain references; add `'static` bound to `U`
fn g<'a, T, U>(v: &'a A<U>) -> ~X<'a>: {
~B(v) as ~X<'a>: //~ ERROR value may contain references; add `'static` bound to `U`
}

fn h<'a, T: 'static>(v: ~A<T>) -> ~X: {
~B(v) as ~X: // ok
fn h<'a, T: 'static>(v: &'a A<T>) -> ~X<'a>: {
~B(v) as ~X<'a>: // ok
}

fn main() {}
Expand Down
22 changes: 22 additions & 0 deletions src/test/compile-fail/regionck-uniq-trait-cast-fail.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2014 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Test that lifetimes can't escape through owned trait casts

trait X {}
impl<'a> X for &'a X {}

fn foo(x: &X) -> ~X: {
~x as ~X:
//~^ ERROR lifetime of the source pointer does not outlive lifetime bound of the object type
}

fn main() {
}
27 changes: 27 additions & 0 deletions src/test/compile-fail/regionck-uniq-trait-coerce-fail.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2014 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Test that lifetimes can't escape through owned trait coercions

trait A {}
impl<'a> A for &'a A {}

struct B;
impl A for B {}
impl<'a> A for &'a B {}

fn main() {
let _tmp = {
let bb = B;
let pb = ~&bb; //~ ERROR `bb` does not live long enough
let aa: ~A: = pb;
aa
};
}
22 changes: 22 additions & 0 deletions src/test/run-pass/regionck-uniq-trait-cast-pass.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2014 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Test that trait lifetime params aren't constrained to static lifetime
// when casting something to owned trait

trait X<'a> {}
impl<'a> X<'a> for &'a X<'a> {}

fn foo<'a>(x: &'a X<'a>) -> ~X<'a>: {
~x as ~X<'a>:
}

pub fn main() {
}
28 changes: 28 additions & 0 deletions src/test/run-pass/regionck-uniq-trait-coerce-pass.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2014 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Test that trait lifetime params aren't constrained to static lifetime
// when coercing something to owned trait

trait A<'a> {}
impl<'a> A<'a> for &'a A<'a> {}

struct B;
impl<'a> A<'a> for B {}
impl<'a> A<'a> for &'a B {}

pub fn main() {
let bb = B;
let _tmp = {
let pb = ~&bb;
let aa: ~A: = pb;
aa
};
}

0 comments on commit 4d77945

Please sign in to comment.