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

Add diagnostics for E0172, fix inline message for E0139 #27127

Merged
merged 1 commit into from
Jul 22, 2015

Conversation

AlisdairO
Copy link
Contributor

As title :-)

Part of #24407.

@Manishearth
Copy link
Member

So sorry, I already did this in Manishearth@850e9be .

I'll update the spreadsheet to give myself a global lock over librustc

@Manishearth
Copy link
Member

If you think that one can be improved or this one is better, let me know and we can merge them :)

@AlisdairO
Copy link
Contributor Author

Ah that's a shame :(. Oh well - is E0172 still okay? I could pull that out into a separate PR.

On 139, I think yours is a better explanation of more common scenarios to hit this problem. That said, I think the short error message may be a little off? It talks about type parameters in its interior, but for the following example, I wouldn't really call the parameter 'interior':

fn foo<T: std::fmt::Display()>(bar: T) {
    unsafe {
        let baz: std::raw::TraitObject = std::mem::transmute(bar);
    }
}

@Manishearth
Copy link
Member

Sure, in fact just amending the commit in this PR would work too.

Hmm. "Interior" might be alluding to situations where there are no explicit type params involved.

E.g.

fn foo<T>(x: *const T) -> Box<T>{
type ConcreteRaw = *const T;
type Concrete = Box<T>;
transmute::<ConcreteRaw, Concrete>(x);
}

But this isn't an important distinction, really. "contaisn unsubstituted type parameters" or some such might be better. (Feel free to make the change to the inline error in this same PR)

@AlisdairO AlisdairO changed the title Add diagnostics for E0172 and E0139 Add diagnostics for E0172, fix inline message for E0139 Jul 20, 2015
@AlisdairO
Copy link
Contributor Author

OK, I think that should do it!

```

The code is trying to specify that we want to receive a signed 32-bit integer
which also implements Display. This doesn't make sense: when we pass `i32`, a
Copy link
Member

Choose a reason for hiding this comment

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

nit: backticks around Display

@Manishearth
Copy link
Member

LGTM modulo nit

@AlisdairO
Copy link
Contributor Author

Added the nit-fix, and squashed.

@Manishearth
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 20, 2015

📌 Commit 686d326 has been approved by Manishearth

@GuillaumeGomez
Copy link
Member

@AlisdairO: If you wanna see what error code remains, take a look at the spreadsheet available here : #24407.

@AlisdairO
Copy link
Contributor Author

@GuillaumeGomez Thanks a lot - I've been going through and marking them off in the spreadsheet, but I think we just hit an unfortunate coincidence :-)

@GuillaumeGomez
Copy link
Member

On which one ?

@AlisdairO
Copy link
Contributor Author

On E0139.

@GuillaumeGomez
Copy link
Member

What's about E0139 ? It was marked as merged ?

@AlisdairO
Copy link
Contributor Author

I don't think so - at least, as I see the spreadsheet I'm still marked as the assignee. I see from the issue that I'm supposed to comment in there too, which I haven't been doing, so I'll do that in future!

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 21, 2015
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 21, 2015
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 21, 2015
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 22, 2015
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 22, 2015
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 22, 2015
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 22, 2015
@bors bors merged commit 686d326 into rust-lang:master Jul 22, 2015
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