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

Ignore PhantomData when checking CoerceUnsized implementations #27483

Closed
wants to merge 1 commit into from

Conversation

Gankra
Copy link
Contributor

@Gankra Gankra commented Aug 3, 2015

Fixes #26905

May want to wait on rust-lang/rfcs#1234

I'm not super happy with the duplicated logic in trans and typeck, and would love to know if there's a "better way". It's pretty minimal, regardless.

r? @nrc

// Otherwise, simply copy the data from the source.
assert_eq!(src_ty, target_ty);
if !is_phantom {
assert_eq!(src_ty, target_ty);
Copy link
Member

Choose a reason for hiding this comment

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

assert!(is_phantom || src_ty == target_ty);

@Gankra Gankra force-pushed the aint-fraid-a-no-ghosts branch from 5265376 to d1f94b7 Compare August 3, 2015 02:49
@nrc
Copy link
Member

nrc commented Aug 3, 2015

r=me with an extra test and nits addressed, should wait for the RFC before landing though

@Gankra Gankra force-pushed the aint-fraid-a-no-ghosts branch from d1f94b7 to ff01a42 Compare August 3, 2015 03:09
@Gankra Gankra force-pushed the aint-fraid-a-no-ghosts branch from ff01a42 to 2214483 Compare August 3, 2015 03:10
@Gankra
Copy link
Contributor Author

Gankra commented Aug 3, 2015

nits addressed, I think.

@nrc
Copy link
Member

nrc commented Aug 3, 2015

Thanks! r=me, but waiting on RFC

@eddyb
Copy link
Member

eddyb commented Aug 3, 2015

LGTM. You could deduplicate the check by adding an is_phantom method to middle::ty::Ty.

@bors
Copy link
Contributor

bors commented Aug 7, 2015

☔ The latest upstream changes (presumably #27551) made this pull request unmergeable. Please resolve the merge conflicts.

@Gankra
Copy link
Contributor Author

Gankra commented Sep 14, 2015

superseded

@Gankra Gankra closed this Sep 14, 2015
bors added a commit that referenced this pull request Sep 17, 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