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

Stable regression in tcod-0.8.0 resolving AsRef<Path> #30744

Closed
brson opened this Issue Jan 6, 2016 · 16 comments

Comments

Projects
None yet
7 participants
@brson
Copy link
Contributor

brson commented Jan 6, 2016

Originally reported by crater.

tcod-0.8.0 builds on 1.4 but not 1.5. The errors (two of them) are:

brian@ip-10-235-20-183:~/dev/tcod-0.8.0⟫ cargo build
   Compiling tcod-sys v3.0.2
   Compiling tcod v0.8.0 (file:///mnt/dev/tcod-0.8.0)
src/console.rs:479:39: 479:62 error: mismatched types:
 expected `&std::path::Path`,
    found `&core::convert::AsRef<std::path::Path>`
(expected struct `std::path::Path`,
    found trait core::convert::AsRef) [E0308]
src/console.rs:479                 Root::set_custom_font(self.font_path.as_ref(),
                                                         ^~~~~~~~~~~~~~~~~~~~~~~
src/console.rs:479:39: 479:62 help: run `rustc --explain E0308` to see a detailed explanation
src/console.rs:486:60: 486:70 error: no method named `as_bytes` found for type `&core::convert::AsRef<str>` in the current scope
src/console.rs:486             let c_title = CString::new(self.title.as_ref().as_bytes()).unwrap();
                                                                              ^~~~~~~~~~
error: aborting due to 2 previous errors
Could not compile `tcod`.
@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Jan 6, 2016

You can get the source with curl -sSfL https://crates.io/api/v1/crates/tcod/0.8.0/download > tcod-0.8.0.tar.gz. Note that you'll have to edit Cargo.toml to remove the path line in the tcod-sys dep.

@brson brson changed the title Regression in tcod-0.8.0 resolving AsRef<Path> Stable regression in tcod-0.8.0 resolving AsRef<Path> Jan 6, 2016

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Jan 6, 2016

I've updated the OP to indicate that this is actually a regression from 1.4->1.5 stable.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 6, 2016

likely related to adding an AsRef impl?

cc @alexcrichton

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Jan 6, 2016

Note that self.font_path has type Box<AsRef<Path> + 'a>, and the error we're getting here is connected to some intermediate trait object.

I don't think this is related to new AsRef impls or anything like that. Possibly an issue with method dispatch choking on the multiple layers at play here?

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Jan 6, 2016

We can try to bisect.

@brson brson self-assigned this Jan 6, 2016

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Jan 6, 2016

@brson OK -- FWIW, the current behavior looks buggy, so we can can also just try to reduce the test and make a fix directly.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 7, 2016

Probably a good starting point would be trying to create a minimized test case.

@bltavares

This comment has been minimized.

Copy link
Contributor

bltavares commented Jan 7, 2016

Would this be a minimized test case?

use std::path::Path;

struct Root<'a> {
    font_path: Box<AsRef<Path> + 'a>
}

fn main() {
    let path = Path::new("/");
    let root = Root {
        font_path: Box::new(&path)
    };

    let path_ref : &Path = root.font_path.as_ref();
}

I get the following with rustc 1.7.0-nightly (d5e229057 2016-01-04)

30744.rs:13:28: 13:51 error: mismatched types:
 expected `&std::path::Path`,
    found `&core::convert::AsRef<std::path::Path>`
(expected struct `std::path::Path`,
    found trait core::convert::AsRef) [E0308]
30744.rs:13     let path_ref : &Path = root.font_path.as_ref();
                                       ^~~~~~~~~~~~~~~~~~~~~~~
30744.rs:13:28: 13:51 help: run `rustc --explain E0308` to see a detailed explanation
error: aborting due to previous error
@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Jan 7, 2016

triage: P-high

someone should indeed try to figure out what the problem is; whether the test case in the previous comment is a representative (i.e. try it against Rust 1.4 or so), et cetera.

@rust-highfive rust-highfive added P-high and removed I-nominated labels Jan 7, 2016

@bltavares

This comment has been minimized.

Copy link
Contributor

bltavares commented Jan 7, 2016

I was able to confirm that the code fails to compile on rustc 1.5.0 (3d7cd77e4 2015-12-04) but compiles on rustc 1.4.0 (8ab8581f6 2015-10-27).

On January 7, 2016 7:07:41 PM GMT-02:00, Felix S Klock II notifications@github.com wrote:

triage: P-high

someone should indeed try to figure out what the problem is; whether
the test case in the previous comment is a representative (i.e. try it
against Rust 1.4 or so), et cetera.


Reply to this email directly or view it on GitHub:
#30744 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Jan 7, 2016

@bltavares Thanks for the investigation!

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Jan 7, 2016

I'm going to look into this.

@arielb1 arielb1 added the T-libs label Jan 8, 2016

@arielb1

This comment has been minimized.

Copy link
Contributor

arielb1 commented Jan 8, 2016

I am quite sure this impl is guilty:

#[stable(since = "1.5.0", feature = "smart_ptr_as_ref")]
impl<T: ?Sized> AsRef<T> for Box<T> {
    fn as_ref(&self) -> &T {
        &**self
    }
}
@arielb1

This comment has been minimized.

Copy link
Contributor

arielb1 commented Jan 8, 2016

I am quite sure we have seen this issue before - and our consensus is that methods like as_ref should not be used in non-generic code, because autoderef may start picking the wrong method. This is however an annoying gotcha in Rust. Better add it to the FAQ.

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Jan 8, 2016

@arielb1 Good catch; I'm sure you're correct, and I agree with the analysis. Thanks!

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Jan 8, 2016

Thanks everyone for investigating.

@brson brson closed this Jan 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.