Uh oh!
There was an error while loading. Please reload this page.
test: Add a test for variance in trait matching. - #15191
Conversation
There was a problem hiding this comment.
This doesn't need unsafe because the effect pass runs after the type checker?
huonw
commented
Jun 26, 2014
Also, is this test case correct w.r.t. to the original issue? It was originally I removed my r+ temporarily. |
pcwalton
commented
Jun 26, 2014
@huonw Yeah, you're right, it's broken and this is still a bug. Looking into it. |
pcwalton
commented
Jun 26, 2014
@huonw Should be fixed now for real. r? |
huonw
commented
Jun 26, 2014
Travis failed. |
edwardw
commented
Jun 26, 2014
I think this patch may have only addressed the vtable part of #5781. There is another half, namely in librustc/middle/typeck/infer/combine.rs#L92, it is still contravariant only for self type. IIRC, if we do fix it, then bivariant weirdness starts to creep in. That's what rfc#12 is for, but now that rfc hasn't made progress for a while. As for the new |
pcwalton
commented
Jun 26, 2014
@edwardw Thanks for the heads up! Looks like fixing |
pcwalton
commented
Jun 26, 2014
r? @huonw |
pcwalton
commented
Jun 26, 2014
I have removed all FIXME's about that issue and made the changes suggested. |
pcwalton
commented
Jun 28, 2014
It's unfortunate, yes, but I think that being able to return arbitrary references in safe code is much more unfortunate. I believe that this bug allows you to transmute arbitrary values to any other arbitrary value in safe code. Let's lock it down for now and see how we can address the usability problem later. re-r? @huonw |
This can break code that looked like:
impl Foo for Box<Any> {
fn f(&self) { ... }
}
let x: Box<Any + Send> = ...;
x.f();
Change such code to:
impl Foo for Box<Any> {
fn f(&self) { ... }
}
let x: Box<Any> = ...;
x.f();
That is, upcast before calling methods.
This is a conservative solution to rust-lang#5781. A more proper treatment (see
the xfail'd `trait-contravariant-self.rs`) would take variance into
account. This change fixes the soundness hole.
Some library changes had to be made to make this work. In particular,
`Box<Any>` is no longer showable, and only `Box<Any+Send>` is showable.
Eventually, this restriction can be lifted; for now, it does not prove
too onerous, because `Any` is only used for propagating the result of
task failure.
This patch also adds a test for the variance inference work in rust-lang#12828,
which accidentally landed as part of DST.
Closesrust-lang#5781.
[breaking-change]huonw
commented
Jun 30, 2014
This has had a few "surprising" consequences. Consider the following code traitFoo:Clone{}implFooforfn(&int){}fnfoo(_:&int){}fnmain(){
foo.clone();}This compiles fine with 9f8149e (before this landed), but fails with 1e6b699 (the version currently on the playpen): http://is.gd/qkFy3n Of particular note is that commenting out the There's another example where trait impls like impl<'a,T:HasTransform<'a,Matrix2d>
+ CanTransform<'a,T,Matrix2d>>RelativeTransform2d<'a>forT{fn trans(&'aself,x:Scalar,y:Scalar) -> T{cannot be used like: pubstructContext<'a>{ ... }fnfoo(){let c = Context::new();{let d = c.trans(20.0,40.0);let _d = d.trans(10.0,10.0);}}because the (I'm not suggesting at all that this patch should be reverted, just listing two corner-cases people have met due to this, for the record.) |
bvssvni
commented
Jun 30, 2014
@huonw reduced my example down to: http://is.gd/Yl44ye pubstructContext<'a>{pubview:Option<&'aint>,}pubtraitRelativeTransform2d<'a>{fntrans(&'aself) -> Self;}/*// WORKS WHEN IMPLEMENTING TRAIT DIRECTLYimpl<'a> RelativeTransform2d<'a> for Context<'a> { fn trans(&'a self) -> Context<'a> { *self }}*/pubtraitCanTransform<'a,T>{fntransform(&'aself) -> T;}impl<'a>CanTransform<'a,Context<'a>>forContext<'a>{fntransform(&'aself) -> Context<'a>{*self}}impl<'a,T:CanTransform<'a,T>>RelativeTransform2d<'a>forT{fntrans(&'aself) -> T{self.transform()}}fnmain(){let c = Context{view:None};{let d = c.trans();let _d = d.trans();}} |
alexchandel
commented
Jun 30, 2014
According to@PistonDevelopers, a bug was introduced and mentioned above which breaks rust-graphics in Rust 0.11. @PistonDevelopers asked that 0.11's release is blocked until the bug fixed. It's very possible this is already taken care of, but just in case it wasn't mentioned in the repo... |
bvssvni
commented
Jul 2, 2014
Rust-Graphics has now a work around: PistonDevelopers/graphics#559 |
…f, r=lowr Disable remove unnecessary braces diagnotics for self imports Disable `remove unnecessary braces` diagnostic if the there is a `self` inside the bracketed `use` Fixrust-lang#15191
…ens (rust-lang#15191) Fixesrust-lang/rust-clippy#15008 changelog: [`legacy_numeric_constants`]: fix suggestion when call is inside parentheses like `(i32::max_value())`
I believe that #5781 got fixed by the DST work. It duplicated the
variance inference work in #12828. Therefore, all that is left in #5781
is adding a test.
Closes#5781.
r? @huonw