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

Remove the unneeded Sized bound on TypeId creation #20662

Merged
merged 1 commit into from
Jan 7, 2015

Conversation

reem
Copy link
Contributor

@reem reem commented Jan 6, 2015

This bound is probably unintentional and is unnecessarily
constricting.

This bound is probably unintentional and is unnecessarily
constricting.
@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@nrc
Copy link
Member

nrc commented Jan 6, 2015

Why is it constricting, it admits more concrete types for T?

@nrc
Copy link
Member

nrc commented Jan 6, 2015

crap, ignore me

@nikomatsakis
Copy link
Contributor

Seems reasonable.

@alexcrichton
Copy link
Member

Just a friendly remind to be sure to run tests! (this ICEs in stage0 libcore, the very first thing we compile)

@reem
Copy link
Contributor Author

reem commented Jan 7, 2015

@alexcrichton Weird, I ran make check-stage1-std with no errors locally.

EDIT: I may have misused the build system... further investigation pending.

@alexcrichton
Copy link
Member

Ah I registered a new snapshot locally which must have triggered the error.

@reem
Copy link
Contributor Author

reem commented Jan 7, 2015

@alexcrichton Nope, this does ICE. I did wrong things to the build system.

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jan 7, 2015
This bound is probably unintentional and is unnecessarily
constricting.
@alexcrichton alexcrichton merged commit 2404232 into rust-lang:master Jan 7, 2015
@alexcrichton
Copy link
Member

This wasn't actually merge (sorry didn't rebase this out), but feel free to reopen with the ICE fixed!

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.

5 participants