Skip to content

Conversation

nrc
Copy link
Member

@nrc nrc commented Jun 13, 2014

Use ty_rptr/ty_uniq(ty_trait) rather than TraitStore to represent trait types.
Also addresses (but doesn't close) #12470.
Part of the work towards DST (#12938).

@nrc
Copy link
Member Author

nrc commented Jun 13, 2014

Interesting test failure...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two spaces between substs and bounds.builtin_bounds. This might show up in other places, too.

@nrc
Copy link
Member Author

nrc commented Jun 15, 2014

r? anyone?

@nrc
Copy link
Member Author

nrc commented Jun 15, 2014

Wut? That doesn't happen locally :-(

@nikomatsakis
Copy link
Contributor

/me will read

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking about this lately, as we briefly discussed on the phone. On the one hand, there are rules against assigning lvalues of unsigned type, so in fact an &mut U(where U is unsized) is more limited than an &mut S (where S is sized). In particular, you cannot do *x = foo as Trait or something like that. For this reason, I believe that e.g. &mut (Trait1+Trait2) <: &mut Trait1 is sound, though I want to consider it more deeply. Nonetheless, I am not happy about the rules for &mut T considering whether T is sized.

I am not convinced this is related to #12470 -- can you open a separate bug perhaps to settle variance w/r/t DST and &mut more definitively?

Also, I presume that when you didn't have this code, various things broke?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, at first I thought this was sound too. I couldn't come up with a counter-example. So now I'm not sure.

#12470 is about enforcing invariance here for lifetimes, whereas this is about enforcing invariance for bounds. I'll open a new bug for issue.

Yep, stuff broke. Not sure how much.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #14985

Use ty_rptr/ty_uniq(ty_trait) rather than TraitStore to represent trait types.
Also addresses (but doesn't close) rust-lang#12470.
Part of the work towards DST (rust-lang#12938).

[breaking-change] lifetime parameters in `&mut trait` are now invariant. They used to be contravariant.
bors added a commit that referenced this pull request Jun 18, 2014
Use ty_rptr/ty_uniq(ty_trait) rather than TraitStore to represent trait types.
Also addresses (but doesn't close) #12470.
Part of the work towards DST (#12938).
@bors bors closed this Jun 18, 2014
@bors bors merged commit 8e7213f into rust-lang:master Jun 18, 2014
@nrc nrc deleted the tstore branch June 18, 2014 02:37
@nrc nrc mentioned this pull request Jun 18, 2014
23 tasks
edwardw added a commit to edwardw/rust that referenced this pull request Jun 23, 2014
The rust-lang#14869 removed `TraitStore` from `ty_trait` and represented trait
reference as regular `ty_rptr`. An old bug of the missing constraint
upon lifetime parameter of trait reference then is fixed as a side
effect. Adds tests for affected bugs and closes them.

Closes rust-lang#12470.
Closes rust-lang#14285.
alexcrichton pushed a commit to alexcrichton/rust that referenced this pull request Jun 25, 2014
The rust-lang#14869 removed `TraitStore` from `ty_trait` and represented trait
reference as regular `ty_rptr`. An old bug of the missing constraint
upon lifetime parameter of trait reference then is fixed as a side
effect. Adds tests for affected bugs and closes them.

Closes rust-lang#12470.
Closes rust-lang#14285.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants