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

Combine types and regions in Substs into one interleaved list. #36002

Merged
merged 3 commits into from
Aug 27, 2016

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Aug 25, 2016

Previously, Substs would contain types and regions, in two separate vectors, for example:

<X as Trait<'a, 'b, A, B>>::method::<'p, 'q, T, U>
/* corresponds to */
Substs { regions: ['a, 'b, 'p, 'q], types: [X, A, B, T, U] }

This PR continues the work started in #35605 by further removing the distinction.
A new abstraction over types and regions is introduced in the compiler, Kind.
Each Kind is a pointer (&TyS or &Region), with the lowest two bits used as a tag.
Two bits were used instead of just one (type = 0, region = 1) to allow adding more kinds.

Substs contain only a Vec<Kind>, with Self first, followed by regions and types (in the definition order):

Substs { params: [X, 'a, 'b, A, B, 'p, 'q, T, U] }

The resulting interleaved list has the property of being the concatenation of parameters for the (potentially) nested generic items it describes, and can be sliced back into those components:

params[0..5] = [X, 'a, 'b, A, B] // <X as Trait<'a, 'b, A, B>>
params[5..9] = ['p, 'q, T, U] // <_>::method::<'p, 'q, T, U>

r? @nikomatsakis

impl<'tcx> From<Ty<'tcx>> for Kind<'tcx> {
fn from(ty: Ty<'tcx>) -> Kind<'tcx> {
// Ensure we can use the tag bits.
assert_eq!(mem::align_of_val(ty) & TAG_MASK, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this really need to be a regular assertion? A debug assertion might be more appropriate (do the nightlies have debug assertions enabled?).

Copy link
Member Author

Choose a reason for hiding this comment

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

If it's not optimized out, then something's wrong. This would be a static_assert if we had those.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, didn't think of that.

@nikomatsakis
Copy link
Contributor

OK. This is pretty slick. Here is one last random thought:

It might be nice to have an enum KindEnum or something and the option to convert a Kind into an enum, so we can exhaustively match on the kinds. That said, I only noticed a few places that wanted to do that, and it seems like the current technique of just writing a big if chain and calling bug is maybe ok. Kinda grody.

But otherwise -- yeah, this is great. r=me modulo nits and suggestions.

@eddyb
Copy link
Member Author

eddyb commented Aug 26, 2016

@nikomatsakis I seriously considered that and realized it wouldn't help much, maybe even cause deoptimizations due to "re-tagging" (I saw some examples in Servo where LLVM didn't see the connection).

@nikomatsakis
Copy link
Contributor

@eddyb I was not expecting it to yield more efficient code, just more correct code. IOW, it'd be harder to miss a case -- but I think that all or most the places I saw where we were doing exhaustive checks are places that you are very unlikely to miss anyway (that is, you will for sure hit e.g. encoding/substitution when writing test cases)

@nikomatsakis
Copy link
Contributor

@eddyb so tl;dr feel free to leave it as is.

@eddyb eddyb force-pushed the abstract-kindness branch from d03b762 to 7a8d482 Compare August 26, 2016 22:15
@eddyb
Copy link
Member Author

eddyb commented Aug 27, 2016

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Aug 27, 2016

📌 Commit 7a8d482 has been approved by nikomatsakis

Manishearth added a commit to Manishearth/rust that referenced this pull request Aug 27, 2016
…akis

Combine types and regions in Substs into one interleaved list.

Previously, `Substs` would contain types and regions, in two separate vectors, for example:
```rust
<X as Trait<'a, 'b, A, B>>::method::<'p, 'q, T, U>
/* corresponds to */
Substs { regions: ['a, 'b, 'p, 'q], types: [X, A, B, T, U] }
```

This PR continues the work started in rust-lang#35605 by further removing the distinction.
A new abstraction over types and regions is introduced in the compiler, `Kind`.
Each `Kind` is a pointer (`&TyS` or `&Region`), with the lowest two bits used as a tag.
Two bits were used instead of just one (type = `0`, region = `1`) to allow adding more kinds.

`Substs` contain only a `Vec<Kind>`, with `Self` first, followed by regions and types (in the definition order):
```rust
Substs { params: [X, 'a, 'b, A, B, 'p, 'q, T, U] }
```
The resulting interleaved list has the property of being the concatenation of parameters for the (potentially) nested generic items it describes, and can be sliced back into those components:
```rust
params[0..5] = [X, 'a, 'b, A, B] // <X as Trait<'a, 'b, A, B>>
params[5..9] = ['p, 'q, T, U] // <_>::method::<'p, 'q, T, U>
```

r? @nikomatsakis
bors added a commit that referenced this pull request Aug 27, 2016
Rollup of 7 pull requests

- Successful merges: #35124, #35877, #35953, #36002, #36004, #36005, #36014
- Failed merges:
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