Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Owned trait objects are erroneously permitted to contain references #11971

Closed
chris-morgan opened this issue Feb 1, 2014 · 8 comments
Closed
Assignees
Labels
A-lifetimes Area: Lifetimes / regions A-type-system Area: Type system
Milestone

Comments

@chris-morgan
Copy link
Member

(pnkfelix: Here is a test case. This behavior is not sound.)

pub trait IntMaker { fn i(&self) -> int; }

impl<'a> IntMaker for &'a int {
    fn i(&self) -> int { let & &x = self; x }
}

#[cfg(does_not_and_should_not_compile)]
fn main_forbidden_soundly() -> ~IntMaker {
    let x = 2;
    ~&x as ~IntMaker
}

fn main_unsound() -> ~IntMaker: {
    let x = 3;
    ~&x as ~IntMaker:
}

pub fn main() {
    let m2 = main_unsound();
    println!("m2: {}", m2.i());
}

Transcript of a run:

% rustc --version
/Users/fklock/opt/rust-dbg/bin/rustc 0.10-pre (caf17fe 2014-03-21 02:21:50 -0700)
host: x86_64-apple-darwin
% rustc  /tmp/demo.rs && ./demo
m2: 140306048614800

Original bug report follows

Consider the errors you get with trying to use references inside a managed trait object such as @Any: (remove the : and you get a 'static bound failure error plus the same error):

<anon>:1:38: 1:40 error: value may contain references
<anon>:1 #[feature(managed_boxes)];fn main(){@&2 as @Any:;}
                                              ^~
error: aborting due to previous error

Now, the problem: owned trait objects do not have this contains-references check.

With the default constraints, ~&2 as ~Any does not compile, because something with references is not Send. This is good and proper.

On the other hand, ~&2 as ~Any: does compile successfully. (Complete example: fn main() { ~&2 as ~Any:; }, should not compile.) This is unsound and should be forbidden in the same way that @T: forbids references.

@huonw
Copy link
Member

huonw commented Feb 1, 2014

Actually, I misspoke on IRC: it's @ that is disallowing references (e.g. fn main() { @&1; } will fail to compile for the same reason).

In any case, ~Trait: does need to be fixed.

cc @nikomatsakis

@chris-morgan
Copy link
Member Author

Yeah, I should have seen that myself—the span does show the error as the @&2 part.

@flaper87
Copy link
Contributor

flaper87 commented Feb 1, 2014

cc me

@nikomatsakis
Copy link
Contributor

In some way this is related to #5121. I think what we want is to support ~Trait:'r where 'r is a lifetime bound. It's possible that ~Trait: is doing the right thing, in fact, it depends a bit on the supporting code. I suspect though that there are various bugs in that area. It's been on the "needs attention" list for a while. (as witnessed by the fact that #5121 is not yet fixed)

@pnkfelix
Copy link
Member

still reproduces. Updating description with concrete test case.

dmski added a commit to dmski/rust that referenced this issue Apr 8, 2014
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
@nikomatsakis nikomatsakis self-assigned this May 1, 2014
@pnkfelix
Copy link
Member

pnkfelix commented May 1, 2014

assigning 1.0 milestone, P-backcompat-lang.

@pnkfelix pnkfelix added this to the 1.0 milestone May 1, 2014
@alexcrichton
Copy link
Member

cc #5723, #9745

@pcwalton
Copy link
Contributor

Nominating for closing as a dupe of #5723.

@brson brson closed this as completed Jun 19, 2014
flip1995 pushed a commit to flip1995/rust that referenced this issue Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: Lifetimes / regions A-type-system Area: Type system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants