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

Beta Regression: cannot reference Self from where clause for inherent impl #24944

Closed
bytwise opened this issue Apr 29, 2015 · 18 comments
Closed
Labels
P-medium Medium priority T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@bytwise
Copy link
Contributor

bytwise commented Apr 29, 2015

The following code compiles with rustc 1.0.0-beta (9854143cb 2015-04-02) (built 2015-04-02) but does not compile with rustc 1.0.0-beta.2 (e9080ec39 2015-04-16) (built 2015-04-16)

trait Foo {
    fn foo(&self);
}

struct Bar<T>(T);

impl<T> Bar<T> where Self: Foo {}

fn main() {}
@pnkfelix
Copy link
Member

Specifically, the error message one gets (at least with my beta build) is:

% ./x86_64-apple-darwin/stage1/bin/rustc /tmp/r.rs
/tmp/r.rs:7:22: 7:26 error: use of undeclared type name `Self`
/tmp/r.rs:7 impl<T> Bar<T> where Self: Foo {}
                                 ^~~~
error: aborting due to previous error

@pnkfelix
Copy link
Member

However, this may be deliberate. At least, I don't remember what the rules are about when Self is in scope, if at all, within impls -- I tend to only use it within trait definitions, myself.

In other words, one can certainly workaround this issue by writing the above impl as:

impl<T> Bar<T> where Bar<T>: Foo {}

@bombless
Copy link
Contributor

No way, this is definitely wrong.
Self is a keyword, the compiler should never print error message like that.

@pnkfelix
Copy link
Member

@bombless there's a difference nonetheless between a poor error message and an (unintentional) regression in the compiler itself. I'm trying to identify exactly what this is.

@pnkfelix
Copy link
Member

(I have a strong suspicion that this change was injected by PR #23998, which was a fix for #23909. I am doing some builds to try to verify that suspicion now.)

@pnkfelix
Copy link
Member

(no, from locally building at 926f38e it seems like PR #23998 is not the source of the "regression" here.)

Update: above was based on faulty input test case; i need to go back and check it again.

Update 2: Okay, it was definitely injected by 926f38e and thus due to PR #23998.

@Stebalien
Copy link
Contributor

It is according to rust-lang/rfcs#522.

@pnkfelix
Copy link
Member

@Stebalien it is (what) according to rust-lang/rfcs#522 ? Supposed to be allowed? Behaving as specified?

Update: In particular, there is plenty of wiggle room in rust-lang/rfcs#522 as written. It just says "When used inside an impl ..." without formally defining what "inside an impl" actually means, and thus a reasonable person might think it means "inside the curly braces associated with the impl"

Having said that, I am inclined to infer that you meant that RFC 522 is supposed to imply that this usage of Self is legal, and I personally am inclined to agree with that (inferred) opinion.

@arielb1
Copy link
Contributor

arielb1 commented Apr 29, 2015

@bombless

This is just the standard error message you get when you try to use Self outside of a trait, e.g.

fn foo(_: Self) {}

@Stebalien
Copy link
Contributor

@Stebalien it is (what) according to rust-lang/rfcs#522 ? Supposed to be allowed? Behaving as specified?

IMHO, it should be allowed.

@pnkfelix
Copy link
Member

cc @nrc : was this an intentional side-effect of #23998, or is this an inadvertent bug that we should fix?

(If its a bug, I'm assuming we can prioritizing fixing it based on how much breakage we observe on crates.io ?)

@pnkfelix pnkfelix changed the title Beta Regression Beta Regression: cannot reference Self from where clause for inherent impl Apr 29, 2015
@bombless
Copy link
Contributor

@arielb1
You're right, filed an issue #24968

@nrc
Copy link
Member

nrc commented Apr 30, 2015

@pnkfelix it was not deliberate. I don't really have an opinion on whether it is right or wrong as stands, I guess if you can write a bound on the concrete version of Self, you should be able to use Self itself.

@pnkfelix
Copy link
Member

triage: I-nominated

I want to figure out how important it is to fix this.

I suspect we can get away with just treating it as "just a bug" -- that is, I have not found a way to utilize it in a way that it would be a breaking-change to fix it. For example, something like this, where Self might be referring to an outer type, does not compile today (and thus gives me confidence that we can fix this tomorrow):

    trait A { fn foo(&self) { } }
    struct S;
    struct T;
    impl A for S { }
    impl S {
        fn foo(&self) {
            impl T where Self: A { }
        }
    }

@pnkfelix
Copy link
Member

triage: P-medium

@rust-highfive rust-highfive added P-medium Medium priority and removed I-nominated labels Apr 30, 2015
@steveklabnik
Copy link
Member

Triage: these examples still error, though with slightly different messages.

@steveklabnik steveklabnik added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed A-lang labels Mar 24, 2017
@Mark-Simulacrum
Copy link
Member

Re-nominating: The issue description compiles, though as far as I can tell, it probably shouldn't, since Self: Foo isn't being met -- and cannot be met, I think, due to coherence. Perhaps this is working as intended though. We should decide and either close this (if yes) or keep it open if no.

trait Foo {
    fn foo(&self);
}

struct Bar<T>(T);

impl<T> Bar<T> where Self: Foo {}

fn main() {}

@nikomatsakis
Copy link
Contributor

Behaving as expected. At some point we decided that Self, in an inherent impl, referred to the type on which the impl is defined, which is what is happening here. It's ok that the where-clause cannot be fulfilled -- it might be implemented in a downstream crate, for eample.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium Medium priority T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants