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

Introduce UnpackedKind #48452

Merged
merged 2 commits into from
Feb 25, 2018
Merged

Introduce UnpackedKind #48452

merged 2 commits into from
Feb 25, 2018

Conversation

varkor
Copy link
Member

@varkor varkor commented Feb 23, 2018

This adds an UnpackedKind type as a typesafe counterpart to Kind. This should make future changes to kinds (such as const generics!) more resilient, as the type-checker will be able to catch more potential issues.

r? @eddyb
cc @yodaldevoid

This adds an `UnpackedKind` type as a typesafe counterpart to `Kind`. This should make future changes to kinds (such as const generics!) more resilient, as the type-checker should catch more potential issues.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 23, 2018
TYPE_TAG => unsafe {
UnpackedKind::Type(&*((ptr & !TAG_MASK) as *const _))
},
_ => bug!("packed kind has invalid tag")
Copy link
Member

Choose a reason for hiding this comment

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

cc @nikomatsakis Should this be bug!? That seems expensive. intrinsics::unreachable() should be fine since Kind can't be safely created with a different tag, and would avoid unnecessary cases in matches.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that unreachable! in this case seems ok, since we're meddling in unsafe code, and this is inside the abstraction barrier. Plus this is "inner loop" code.

@eddyb
Copy link
Member

eddyb commented Feb 23, 2018

r=me when the fallback case in unpacked is resolved

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

❤️

Ok(Kind::from(relation.relate_with_variance(variance, &a_r, &b_r)?))
} else {
bug!()
match (a.unpack(), b.unpack()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice, I've been wanting this to be done forever =)

TYPE_TAG => unsafe {
UnpackedKind::Type(&*((ptr & !TAG_MASK) as *const _))
},
_ => bug!("packed kind has invalid tag")
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that unreachable! in this case seems ok, since we're meddling in unsafe code, and this is inside the abstraction barrier. Plus this is "inner loop" code.

@eddyb
Copy link
Member

eddyb commented Feb 23, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Feb 23, 2018

📌 Commit f2b9686 has been approved by eddyb

@bors
Copy link
Contributor

bors commented Feb 23, 2018

🌲 The tree is currently closed for pull requests below priority 99, this pull request will be tested once the tree is reopened

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 23, 2018
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 24, 2018
Introduce UnpackedKind

This adds an `UnpackedKind` type as a typesafe counterpart to `Kind`. This should make future changes to kinds (such as const generics!) more resilient, as the type-checker will be able to catch more potential issues.

r? @eddyb
cc @yodaldevoid
bors added a commit that referenced this pull request Feb 25, 2018
Rollup of 15 pull requests

- Successful merges: #47689, #48110, #48197, #48296, #48386, #48392, #48404, #48415, #48441, #48448, #48452, #48481, #48490, #48499, #48503
- Failed merges:
@bors bors merged commit f2b9686 into rust-lang:master Feb 25, 2018
@varkor varkor deleted the unpacked-kind branch February 25, 2018 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants